I was asked recently about a style choice and whether it was pythonic or not. More to the point, what was the “obvious” way to do this. The question regarded having to run a function before and after another function based on an argument. Here are some options, which do you feel is best practice?
def function_a(arg):
if arg:
otherfunc()
dostuff()
if arg:
otherfunc()
def function_b(arg):
arg and otherfunc()
dostuff()
arg and otherfunc()
def function_c(arg):
if arg:
otherfunc()
dostuff()
otherfunc()
else:
dostuff()
Using and in place of if/then makes perfect sense to me in a functional context, but here I don’t like it at all. Since otherfunc is only being called to elicit some side effects, “if arg evaluates to true, then call otherfunc” is way more consistent with what you’re actually trying to do. However, if otherfunc had a return value that you were using immediately, it’d be more permissible. For example, I’d consider all of the following more or less equally good style (though I’d probably prefer the middle one):
def passes_check(requires_check):
if requires_check:
return check()
return True
def passes_check(requires_check):
return check() if requires_check else True
def passes_check(requires_check):
return (not requires_check) or check()
That rules out your function_b in my book. As for the other two, it’s an issue of code duplication. If all that’s being duplicated is a single function call (as in your example), I’d call it a toss-up. Any more than that, and I’d strongly prefer function_a. In any case, my personal inclination would be your function_a, regardless of how little code is duplicated.
The fact that you’re branching on arg multiple times is quite negligible, because it’s just a value. Of course, if you were calling a function in the condition instead of simply checking the value of arg, then A and B would not be equivalent.
(C) is structured in the clearest and easiest-to-read way hence the most pythonic – although the duplicate function calls muddy the waters a bit, I"m not sure if you want to use those as standings for ‘generic code here’.
Ternary-style operator compression is somewhat controversial in python land: stuff like
value = 10 if a > 0 else 5
Which are very common in c#, say, are unwelcome by many pythonistas. For default assignements and ‘ifnull’ behaviors it’s usually looked on benignly, though:
Value = arg or "Default"
Using the logical operator compression in arg and otherfunc() is rare and not idiomatic - in fact I don’t think I’ve ever seen it in python for flow control.
One of the core python values is readability - saving a line or a few characters should not trump clear expression of code intent. As a style and readability thing, I’d reverse (C):
def D(arg):
if not arg:
dostuff()
return
otherstuff() # i'm omitting the second call to otherstuff on the assumption it's placeholder.
dostuff()
# do otherstuff() again?
The idea here is to get out of the function altogether as early as possible, so people reading the code don’t need to scan to the end to see the control flow. Otherwise you need to read past a lot of code you’re not interested in to get to the code you care about.
My favorite resource for these questions (not just in Python) is The Art of Readable Code:
So the otherstuff function needs to be called twice. For example say otherstuff() touched a bunch of files and these files needed to be touched before and after the dostuff() function. I’ve hit this sort of thing only a few times and haven’t really found a good way to do it without copying code or making the duplicate if statements.
I was just kind of curious if anyone had an elegant solution to this. I would agree that it all comes down to readability and if you are familiar with how python evaluates arg and otherstuff()
you’ll see that if arg is not truthy otherstuff() will never get executed. If you are not familiar with it or forget, then readability is compromised.
in a case like that i’d have dostuff() return a testable value that indicates whether or not to re-call otherstuff():
def F(arg):
if arg:
otherstuff() #precall if arg is true
if dostuff(): # always execute dostuff()
otherstuff() # if dostuff returns true, repeat otherstuff
Is the minimal expression of the logic tree here. Even here, in practice I’d stick a clearly named logic variable to hold the result of dostuff() so that the reader can tell that dostuff() is returning a result that tells the code whether or not to continue: it’s redundant, but expresses the intent more precisely: this is exactly the same as above but easier to grok:
def F(preprocess ):
if preprocess:
otherstuff() #precall if arg is true
post_process = dostuff()
if post_process:
otherstuff()
Second what’s been said already, and personally I’d favor function_c… easier to read. And as they say, code is read far more often than it’s written.