Martin Chilvers | 6 Sep 17:20
Picon

Re: [Traits] feature request

G'day,

Prabhu Ramachandran wrote:
> Martin Chilvers wrote:
>>> Clearly, this will break the trait_set method when 
>>> trait_change_notify=False.  I propose to add a 
>>> _get_trait_change_notify() to the ctraits.c to be able to get this so 
>>> users can write:
>>>
>>>  if self._get_trait_change_notify():
>>>      ...
>>>
>>> etc. to work around this.
>>
>> Hmmm... I wouldn't want to have to put this line in every property 
>> setter... smells like duplication... Couldn't the 
>> 'trait_property_changed' method handle the check?
> 
> Hmm, thats a good point and the patch would be extremely simple too. 
> Just do nothing if the flag has been set.  So here is an updated patch 
> and test case.

Nice!

I don't want to preach to the converted (and I'm not ranting here - for a change!), but this issue 
is an excellent example of API design by thinking about the intent of the code as opposed to the 
technical steps required to get it done...

This is another area where TDD helps out - by forcing you to consider how you would use your API 
first, as opposed to what we sometimes do which is to write the low-level stuff and then see how the 
API evolves... For me, when I'm trying to design an API (by writing a test ;^) the question boils 
down to 'what would I *want* to write to do this task?'... In this case, I am writing a trait setter 
and I want to let people know that the property has changed so, the following seems reasonable 
(although 'fire_trait_property_changed' *might* be clearer ;^):-

class Foo(HasTraits):
     def _set_bar(self, bar):
        """ Trait setter. """

        ...
        self.trait_property_changed(self, old, new)

        return

Whereas the extra check does not, because the intent of the code (i.e. to let people know that the 
property has changed) has been obscured by a low-level conditional check - that the developer would 
not think about, let alone want to write.

class Foo(HasTraits):
     def _set_bar(self, bar):
        """ Trait setter. """

        ...
        if self._get_trait_change_notify():
            self.trait_property_changed(self, old, new)

        return

I'm not criticising here because as developers we often just think about the steps to get something 
done, and its only when somebody else comes to use our code that we realise that the API is maybe a 
bit odd! It is interesting to me that when you stand back and ask the 'what would I want to write 
here?' question, it really seems to help with API designs.. I definitely think that personally 
asking that question has significantly improved my code over the last couple of years (and made me 
cringe when I look back at some of the crap APIs I came up with when letting the API 'bubble up' 
from the low-level details of how things actually get done).

I know this sounds such an obvious and simple idea, its amazing how often it gets overlooked...

Martin

Gmane