code style and readability [was: Re: Checking the boolean value of a collection]

[Larry Bates]

Marco Bizzarri wrote:
> On Sat, Sep 13, 2008 at 4:11 PM, Fredrik Lundh <fredrik at pythonware.com> wrote:
>> Marco Bizzarri wrote:
>>
>>> class FolderInUse:
>>>
>>>    def true_for(self, archivefolder):
>>>        return any([instance.forbid_to_close(archivefolder) for instance in
>>>            self.core.active_outgoing_registration_instances()])
>>>
>>> Is this any better? The true_for name does not satisfy me a lot...
>> well, "true_for" is indeed pretty inscrutable, but I'm not sure that would
>> be the first thing I'd complain about in that verbose mess...
> 
> "verbose mess".
> 
> It is always frustrating when you do what you think is your best and
> you read that.
> 
> Anyway: I'm here to learn, and, of course, part of it is to listen
> those who've been there much longer than you.
> 
> So, thanks for your sincere evaluation, Fredrik :-).
> 
>> (when you pick method names, keep in mind that the reader will see the
>> context, the instance, and the arguments at the same time as they see the
>> name.  there's no need to use complete sentences; pick short short
>> descriptive names instead.)
> 
> Maybe I'm looking at the wrong direction, right now. From the point of
> view of the FolderInUse clients, they will do:
> 
> condition = FolderInUse(core)
> 
> condition.true_for(folder)
> 
> Is this too verbose? This is not a polemic statement, I'm really
> asking your opionion.
> 
> The expression inside the true_for is indeed complex, and maybe I can
> simplify it; however, I'm deeply convinced that
> 
> instance.forbid_to_close(folder)
> 
> has some good points on it; I mean, once I read this kind of code, I
> can hope to understand it without looking at what forbid_to_close
> does.
> 
> 
>> </F>
>>
> 

 >>> class FolderInUse:
 >>>
 >>>    def true_for(self, archivefolder):
 >>>        return any([instance.forbid_to_close(archivefolder) for instance in
 >>>            self.core.active_outgoing_registration_instances()])

IMHO it reads better if you use the __call__ method of the class to return the 
value and rewrite it as a regular loop for clarity.  Sometimes the simplest way 
is the easiest to read.

class FolderInUse:
    def __call__(self, archivefolder):
        result = False
        for instance in self.core.active_outgoing_registration_instances():
            if instance.forbid_to_close(archivefolder):
                result = True
                break

        return result


Then it can be called with:

     if FolderInUse(archivefolder):
         ...

-Larry