CODEWITHSUNDEEP
pythoncoding-stylerefactoringif-statement
D K
D K

Reputation: 5760

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?

Upvotes: 7

Views: 11016

Answers (5)

Ben James
Ben James

Reputation: 125157

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]()

Upvotes: 12

agf
agf

Reputation: 176780

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.

Upvotes: 8

orlp
orlp

Reputation: 117681

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.

Upvotes: 2

Andrew Clark
Andrew Clark

Reputation: 208475

What you are doing now is not bad programming practice, but it would be bad practice to duplicate code by putting the three function calls in each if statement.

Some people prefer to have a single exit point to their functions, in which case I would suggest something like this:

def some_function(eggs):
    error_code = 0
    if eggs == 1:
        do_something_1()
    elif eggs == 2:
        do_something_2()
    elif eggs == 3:
        do_something_3()
    else:
        do_error()
        error_code = 1

    if error_code == 0:
        do_something_4()
        do_something_5()
        do_something_6()
    return   # return error_code if it would be helpful to the calling function

Upvotes: 1

Ned Batchelder
Ned Batchelder

Reputation: 375574

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()

Upvotes: 3

Related Questions