Return and set

[Terry Reedy]

On 7/19/2011 9:52 AM, Billy Mays wrote:
> On 07/19/2011 09:43 AM, Ben Finney wrote:
>> Billy Mays
>> <81282ed9a88799d21e77957df2d84bd6514d9af6 at myhashismyemail.com> writes:
>>
>>> I have a method getToken() which checks to see if a value is set, and
>>> if so, return it. However, it doesn't feel pythonic to me:
>>
>> Clearly that's because the function name is not Pythonic :-)
>>
>> I'll assume the name is a PEP-8 compatible ‘get_token’.
>>
>>> def getToken(self):
>>> if self.tok:
>>> t = self.tok
>>> self.tok = None
>>> return t
>>> # ...
>>
>> Are you testing ‘self.tok’ in a boolean context because you don't care
>> whether it it might be ‘""’ or ‘0’ or ‘0.0’ or ‘[]’ or ‘False’ or lots
>> of other things that evaluate false in a boolean context?
>>
>> If you want to test whether it is any value other than ‘None’, that's
>> not the way to do it. Instead, use ‘if self.token is not None’.
>>
>> But I don't see why you test it at all, in that case, since you're
>> immediately setting it to ‘None’ afterward.
>>
>> Also, the function name is quite misleading; the implication for a
>> function named ‘get_foo’ is that it is a non-destructive read. I would
>> expect the name of this function to indicate what's going on much more
>> explicitly.
>>
>>
>> My suggestion::
>>
>> def get_and_reset_token(self):
>> result = self.token
>> self.token = None
>> return result
>>
>
> This function is used in a file parser. There are two methods,
> getToken() and peekToken(). getToken pops a token from the file, while
> peekToken keeps the token, but still returns it.
>
> Code:
>
> def getToken(self):
> if self.tok:
> t = self.tok
> self.tok = None
> return t
> try:
> t = self.gen.next()
> except StopIteration:
> return NULL
> else:
> return t
>
> def peekToken(self):
> if not self.tok:
> self.tok = self.getToken()
> return self.tok

You did not answer Ben's question about the allowed values of self.tok 
and whether you really want to clobber all 'false' values. The proper 
code depends on that answer.

> NULL is an enumerated value I have defined above. The idea is for
> peekToken to reuse getToken, but to keep the token still around.

I think about reversing and have getToken use peekToken and then reset. 
But that depends on the exact logic which depends on the specs. I would 
more likely have just one function with a reset parameter defaulted to 
the more common value.

-- 
Terry Jan Reedy