Return and set

[Billy Mays]

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:

def getToken(self):
if self.tok:
t = self.tok
self.tok = None
return t
# ...


Is there a way to trim the 'if' block to reset self.tok upon return?

 

[Billy Mays]

Billy Mays

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

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

 

[Terry Reedy]

Billy Mays

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.

 

[Billy Mays]

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.


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.

self.gen is a generator that gets filters single characters from a file.
Values that come from self.gen.next() will always be string values
since the file generator closes on EOF. I can be sure that I will
either get a string from self.gen.next() or catch an exception so its
okay to use NULL (which evaluates to 0).

You are correct it makes more sense to use peekToken() to do the lifting
and getToken() to reuse the token. However, I am not sure what you mean
by reset.

 

[Terry Reedy]

That sounds artificially backwards; why not let getToken() reuse peekToken()?

def peek(self):
if self.tok is None:
try:
self.tok = self.gen.next()

If this is changed (as intended for the iteration protocol) to

self.tok = next(self.gen)

then this code (and OP's version) should run as is on both Py2.6/7 and Py3.