If/then style question

[Steven D'Aprano]

On Thu, 16 Dec 2010 21:49:07 +0000, John Gordon wrote:

> (This is mostly a style question, and perhaps one that has already been
> discussed elsewhere.  If so, a pointer to that discussion will be
> appreciated!)
> 
> When I started learning Python, I wrote a lot of methods that looked
> like this:
> 
>   def myMethod(self, arg1, arg2):
>     if some_good_condition:
>       if some_other_good_condition:
>         if yet_another_good_condition:
>           do_some_useful_stuff()
>           exitCode = good1
>         else:
>           exitCode = bad3
>       else:
>         exitCode = bad2
>     else:
>       exitCode = bad1
>     return exitCode


It doesn't look like you were learning Python. It looks like you were 
learning C with Python syntax :(

The above would be more Pythonically written as:


def myMethod(self, arg1, arg2):
    if not some_good_condition:
        raise SomeException("message")
    if not some_other_good_condition:
        raise SomeOtherException("another message")
    if yet_another_good_condition:
        do_some_useful_stuff()
    else:
        raise SomeThirdException("whatever")


using exceptions to communicate errors out-of-band. Since no return 
result is needed, no explicit return is used and the method is the 
closest thing to a procedure that Python can offer.

The problem with in-band transmission of errors is that they invite the 
anti-pattern of this:

result = obj.myMethod(arg1, arg2)
if result == good1:
    do_something_good()
elif result == bad1:
    handle_error1()
elif result == bad2:
    handle_error2()
elif result == bad3():
    handle_error3()
else:
    print "This can't ever happen, but if it does..."


which all too often becomes:

result = obj.myMethod(arg1, arg2)
if result == good1:
    do_something_good()
else:  # assume result is bad1
    handle_error1()


or even:

who_cares = obj.myMethod(arg1, arg2)
do_something_good()



> But lately I've been preferring this style:
> 
>   def myMethod(self, arg1, arg2):
>     if some_bad_condition:
>       return bad1
>     elif some_other_bad_condition:
>       return bad2
>     elif yet_another_bad_condition:
>       return bad3
>     do_some_useful_stuff()
>     return good1
> 
> I like this style more, mostly because it eliminates a lot of
> indentation.

Well, that's better, but still more like C rather than Python. Avoid the 
anti-pattern of returning in-band error codes. In some languages, either 
exceptions aren't available at all, or the overhead of them is so great 
that for performance you have to avoid them, but Python is not one of 
those languages.

In Python, exceptions are *the* primary way of communicating exceptional 
cases such as (but not limited to) errors. I can only think of two, er, 
exceptions to this rule: str.find() and some of the regular expression 
methods.


> However I recall one of my college CS courses stating that "one entry,
> one exit" was a good way to write code, and this style has lots of
> exits.

Functions always have one entry. The only way to have multiple entry 
points is if the language allows you to GOTO into the middle of a 
function, and Python sensibly does not allow this. The "one entry, one 
exit" rule comes from the days when people would routinely write 
spaghetti code, jumping into and out of blocks of code without using 
functions at all.

If "one entry" is pointless (all functions have one entry!) then "one 
exit" is actively harmful. It leads to adding unnecessary complexity to 
functions, purely to meet the requirements of a rule invented to 
discourage spaghetti code. This hides bugs and increases the maintenance 
burden.

Don't get me wrong... spaghetti code is *bad*. But there are other ways 
of writing bad code too, and hanging around inside a function long after 
you've finished is also bad:

def function(arg):
    done = False
    do_something()
    if some_condition:
        result = "finished"
        done = True
    if not done:
        do_something_else()
        if another_condition:
            result = "now we're finished"
            done = True
    if not done:
        do_yet_more_work()
        if third_condition:
            result = "finished this time for sure"
            done = True
    if not done:
        for i in range(1000000):
            if not done:
                do_something_small()
                if yet_another_condition:
                    result = "finally done!"
                    done = True
    return result
    
It's far more complicated than it need be, and does *lots* of unnecessary 
work. This can be written more simply and efficiently as:

def function(arg):
    do_something()
    if some_condition:
        return "finished"
    do_something_else()
    if another_condition:
        return "now we're finished"
    do_yet_more_work()
    if third_condition:
        return "finished this time for sure"
    for i in range(1000000):
        do_something_small()
        if yet_another_condition:
            return "finally done!"


Over 40% of the code in the first version is scaffolding to support 
skipping the rest of the function body once you have a result. It 
needlessly performs all one million iterations of the loop, even if the 
final result was calculated on the first iteration (loops also should 
have "one entry, one exit"). There's not one good thing that can be said 
about it except that it has "one exit", and that is only a good thing 
compared to the Bad Old Days of writing spaghetti code with GOTO.



-- 
Steven