Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Use of return in long if-elseif-else statements (Python)

I am using Python for my example, but my question is referring to programmming languages in general.

def some_function(eggs):
    if eggs == 1:
        do_something_1()
    elif eggs == 2:
        do_something_2()
    elif eggs == 3:
        do_something_3()
    else:
        do_error()
        return
    do_something_4()
    do_something_5()
    do_something_6()

(This is just an example. My functions will not be called do_something_x.)

Would putting a return in the else like this be a bad programming practice? Would it be a better idea to put

do_something_4()
do_something_5()
do_something_6()

in each of the if/elifs?

like image 461
D K Avatar asked Jul 22 '11 21:07

D K


4 Answers

The main issue I see with your code is that the error case is hidden more than half way down the function body. It makes the code difficult to read. Since what you are doing is validating the arguments to the function, you should do that first.

My preference in the case of an invalid argument is to raise an appropriate exception such as ValueError. Without knowing what your function does, or what do_error does, it's hard to say with absolute certainty that this applies to your case. But generally, getting incorrect arguments isn't something a function can recover from. The caller gave the arguments; so put the onus on the caller to recover from that error.

Also, here's an idiom you can use to avoid long lists of elifs:

funs = {1: do_something_1,
        2: do_something_2,
        3: do_something_3}
funs[eggs]()
like image 153
Ben James Avatar answered Sep 22 '22 18:09

Ben James


Definitely do not copy identical code into each if clause.

How about:

def some_function(eggs):
    options = {1: do_something_1, 2: do_something_2, 3: do_something_3}
    if eggs in options: 
        options[eggs]()
        do_something_4()
        do_something_5()
        do_something_6()
    else:
        do_error()
        return

This doesn't require a long if elif else. It's also clear do_something_4() etc. only happens if eggs is 1, 2, or 3.

like image 36
agf Avatar answered Sep 23 '22 18:09

agf


How about:

def some_function(eggs):
    if eggs not in [1,2,3]:
        do_error()
        return

    if eggs == 1:
        do_something_1()
    elif eggs == 2:
        do_something_2()
    elif eggs == 3:
        do_something_3()
    else:
        assert False
    do_something_4()
    do_something_5()
    do_something_6()
like image 38
Ned Batchelder Avatar answered Sep 22 '22 18:09

Ned Batchelder


Are you really sure that do_something_n is really related to do_something_m?

If so, use do_something(var, n) and use the same code for everything with a few if's (after all the concept is really related, right?).

If not, split the functions into really useful and stand-alone functions.


Example why this (probably) is bad:

def print1():
    print(1)

def print2():
    print(2)

Well, anyone should see this should be printn(n) or something similar.

And the other probability:

def action1():
    paymanagers()
    payworkers()

def action2():
    clean_trashbin()
    unlock_car()

These actions are probably not related and should belong in their own functions.

like image 35
orlp Avatar answered Sep 19 '22 18:09

orlp