question of style

[Duncan Booth]

Simon Forman <sajmikins at gmail.com> wrote:

> Hey I was hoping to get your opinions on a sort of minor stylistic
> point.
> These two snippets of code are functionally identical. Which would you
> use and why?
> The first one is easier [for me anyway] to read and understand, but
> slightly less efficient, while the second is [marginally] harder to
> follow but more efficient.
> 
> ## First snippet
> 
> if self.higher is self.lower is None: return
> if self.lower is None: return self.higher
> if self.higher is None: return self.lower
> 
> ## Second snippet
> 
> if self.higher is None:
>     if self.lower is None:
>         return
>     return self.lower
> if self.lower is None:
>     return self.higher
> 
> What do you think?
> 
> (One minor point: in the first snippet, the "is None" in the first
> line is superfluous in the context in which it will be used, the only
> time "self.lower is self.higher" will be true is when they are both
> None.)
> 

I'd write the first one as:

    	if self.lower is None:
    	    	return self.higher
    	if self.higher is None:
    	    return self.lower

because the entire first 'if' statement is redundant.

As a matter of style however I wouldn't use the shorthand to run two 'is' 
comparisons together, I'd write that out in full if it was actually needed 
here.

Likewise in the second one:

    if self.lower is None:
        return
    return self.lower

is obviously the same as:

    	return self.lower

so apart from reversing the order of the comparisons once you've dropped 
the redundant test it is the same as the first one.