question of style

[Tim Harig]

On 2009-07-02, Duncan Booth <duncan.booth at invalid.invalid> wrote:
> so apart from reversing the order of the comparisons once you've dropped 
> the redundant test it is the same as the first one.

I try to evaluate what you have given regardless of what Booth pointed out.
So, I will only evaluate the first line as it contains most of what I want
to illistrate.

> Simon Forman <sajmikins at gmail.com> wrote:
>> if self.higher is self.lower is None: return

I *really* don't like the fact that you don't specify a return value here
when you have specified a return value for the other conditions.  Obviously
it is possible that something will be looking at the return value for this
function (ie, it would not be a void function() in C).  I had to lookup to
make sure what Python would return in such a case.  If you want the
condition to return something then you should specify what you are
returning.  Specifing 'return None' is much clearer then relying on default
behavior.

I always prefer to see a:
	
	condition:
		operation

structure to my code.  It is the way it is almost always see in Python and
in other languages.  Putting it on one line doesn't save you anything and
it makes it easier to miss something.

I have no particular problem with running the is statements together as an
extended if statement together when it makes it clearer to do so.  The fact
that you want all of them to be the same object is clear.  I do have some
wroblem in this instance because you are referencing "is" against an
instance of a builtin object.

The fact that you used is instead of == for this particular instance
throws me a little because of the use of None.  I wouldn't think second
about it if it was a user defined object.  I have to wonder whether all
objects that are assigned the value of None are actually re-referenced
to the origional None object?  Could deep copy style operations create
two equivilant but different copys of none?  The same problems may be
encounted from any builtin objects such as number literals.

Had None not been specified I would be fine with stringing the comparison
together:

# if all three objects are the same instance, return None
if object1 is object2 is object3:
	return None:

Because of this pecularity, I think that it would be clearer to break the
ifs for simularity and equality:

if object1 is object2:
	if object1 == None:
		return None
	else:
		return object1
else:
	if object1 == None:
		return Object2
	elif object2 == None::
		return Object1
	else:
		# what do we do if neither object == None?
		raise houstonWeHaveAProblemException