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?
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 elif
s:
funs = {1: do_something_1,
2: do_something_2,
3: do_something_3}
funs[eggs]()
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.
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()
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With