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

Question

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?

Ben James · Accepted Answer · 2011-07-22 22:36:59Z

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

Or even funs.get(eggs, do_error)() to handle the case where eggs is not in funs.
By validating the arguments, do you mean if eggs != 1 and eggs != 2 and eggs != 3? Because that isn't very simple if i have 100 options for the value of eggs.
If you use the idiom I suggested, then look at Andrew's comment, or use if eggs not in funs: raise ValueError("Invalid number of eggs")

agf · Accepted Answer · 2011-07-22 22:08:06Z

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.

Ned Batchelder · Accepted Answer · 2011-07-22 22:00:17Z

3

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

answered Jul 22, 2011 at 22:00

Ned Batchelder

378k77 gold badges583 silver badges675 bronze badges

3 Comments

g.d.d.c Over a year ago

I like this answer quite a bit, actually. There are two camps re: early returns: The "all functions should have a single exit point" camp and the "return when you can limit total processing" camp. I tend to side with the latter, especially with python. It seems to simplify functions, reduce indentation, and "feel" cleaner.

SingleNegationElimination Over a year ago

I'm not very fond of the assert as it's used here, you already have the invariant checked explicitly at the top.

dhg Over a year ago

The problem I see with this is that it's actually an example of code duplication -- the exact problem that we want to avoid. Having the options (1,2,3) listed in multiple places opens the possibility of having a mis-match later. What if the author adds an option 4 but doesn't update the "not in"?

orlp · Accepted Answer · 2011-07-22 22:07:13Z

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.

Andrew Clark · Accepted Answer · 2011-07-22 22:05:32Z

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

Those people should use another more functional language, if it's that important to them. Returning early is far cleaner, more readable and maintainable than crufting up the code with error_code-style variables.

Collectives™ on Stack Overflow

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

5 Answers 5

3 Comments

Comments

3 Comments

Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

3 Comments

Comments

3 Comments

Comments

1 Comment

Related