Reputation: 3050
Both functions do the same thing.
def function1(self):
a = self.get_a()
b = self.get_b()
c = self.get_c()
r = None
if a:
r = a
if b:
r = b
if c:
r = c
else:
print("c not set.")
else:
print("b not set.")
else:
print("a not set.")
return r
def function2(self):
a = self.get_a()
b = self.get_b()
c = self.get_c()
r = None
if not a:
print("a not set.")
return r
r = a
if not b:
print("b not set.")
return r
r = b
if not c:
print("c not set.")
r = c
return r
function1() creates very long lines the more if's are nested which conflicts with PEP8's line-length limit of 78.
function2() might be harder to read/understand and has more return statements. Line length is no problem here.
Which one is more pythonic?
Upvotes: 2
Views: 4933
Reputation: 130
Here is what I would do without removing the print statements
def function1(self):
a = self.get_a()
b = self.get_b()
c = self.get_c()
r = None
inputValues = [a, b, c]
setValues = [i for i in inputValues if i]
for index, value in inputValues:
if len(setValues) <= index or setValues[index] != value:
print(f'{value} is not set.')
else:
r = value
return r
The function2
looks good enough to go.
Upvotes: 0
Reputation: 10119
You can use the evaluation rules of the and
and or
operators, for example:
>>> None or 4 or None or 5
4
>>> 4 and 5
5
So you'd have something like:
def function3(self):
a = self.get_a()
b = self.get_b()
c = self.get_c()
return (a and b and c) or (a and b) or a or None
And I'd recommend factoring out you I/O from your logical code.
Upvotes: 1
Reputation: 8569
As @Will's answer suggests, flat is better. However the code doesn't look very pretty anyways. How about a more compact type of code?
looking at these quotes from @Will's answer:
Readability counts.
Beautiful is better than ugly.
from collections import OrderedDict
def function3():
my_dictionary=OrderedDict()
my_dictionary['a'] = self.get_a()
my_dictionary['b'] = self.get_b()
my_dictionary['c'] = self.get_c()
# ...
r = None
for name in my_dictionary.keys():
value = my_dictionary[name]
if not value:
print("%s not set." % name)
return r
r = value
return r
Surely this can be improved even more
Upvotes: 2
Reputation: 3335
I suggest function_4 displayed below together with the questions (non-idetnically working!) functions and one of DomTomCat's answer:
#! /usr/bin/env python
from __future__ import print_function
from collections import OrderedDict # Only used in function_3
def function_4(self):
"""Iterate over call results in FIFO on False or if sequence
exhausted, return None or previous value if that evaluates to true."""
functors = (
self.get_a,
self.get_b,
self.get_c,
)
request_targets = (
'a',
'b',
'c',
)
response_value = None
for functor, request_target in zip(functors, request_targets):
current_response = functor()
if not current_response:
print(request_target, "not set.")
return response_value
else:
response_value = current_response
return response_value
class Foo(object):
"""Mock the thingy ..."""
def __init__(self, a, b, c):
self._a, self._b, self._c = a, b, c
def __repr__(self):
return (
"Foo(" + str(self._a) + ", " + str(self._b) + ", " +
str(self._c) + ")")
def get_a(self):
return self._a
def get_b(self):
return self._b
def get_c(self):
return self._c
def function_1(self):
a = self.get_a()
b = self.get_b()
c = self.get_c()
r = None
if a:
r = a
if b:
r = b
if c:
r = c
else:
print("c not set.")
else:
print("b not set.")
else:
print("a not set.")
return r
def function_2(self):
a = self.get_a()
b = self.get_b()
c = self.get_c()
r = None
if not a:
print("a not set.")
return r
r = a
if not b:
print("b not set.")
return r
r = b
if not c:
print("c not set.")
r = c
return r
def function_3(self):
my_dictionary = OrderedDict()
my_dictionary['a'] = self.get_a()
my_dictionary['b'] = self.get_b()
my_dictionary['c'] = self.get_c()
# ...
r = None
for name in my_dictionary.keys():
value = my_dictionary[name]
if not value:
print("%s not set." % name)
return r
r = value
def main():
""""Drive the investigation."""
fixtures = (
(1, 42, 3.1415),
(0, 42, 3.1415),
(1, 0, 3.1415),
(1, 42, 0),
)
functors = (
function_1,
function_2,
function_3,
function_4,
)
for fixture in fixtures:
foo = Foo(*fixture)
print("\nFixture:", foo)
for i, functor in enumerate(functors, start=1):
print("Functor[%d]:" % (i,))
print(functor(foo))
if __name__ == '__main__':
main()
On my machine the fixtures produce the following behaviour/output when being called:
Fixture: Foo(1, 42, 3.1415)
Functor[1]:
3.1415
Functor[2]:
3.1415
Functor[3]:
None
Functor[4]:
3.1415
Fixture: Foo(0, 42, 3.1415)
Functor[1]:
a not set.
None
Functor[2]:
a not set.
None
Functor[3]:
a not set.
None
Functor[4]:
a not set.
None
Fixture: Foo(1, 0, 3.1415)
Functor[1]:
b not set.
1
Functor[2]:
b not set.
1
Functor[3]:
b not set.
1
Functor[4]:
b not set.
1
Fixture: Foo(1, 42, 0)
Functor[1]:
c not set.
42
Functor[2]:
c not set.
0
Functor[3]:
c not set.
42
Functor[4]:
c not set.
42
[Finished in 0.0s]
Upvotes: 0
Reputation: 24699
One of the principals of Pythonic code is "flat is better than nested". On this basis, I'll say function2()
is objectively more Pythonic. This can be seen in PEP-20: The Zen of Python:
The Zen of Python
Beautiful is better than ugly. Explicit is better than implicit. Simple is better than complex. Complex is better than complicated. Flat is better than nested. Sparse is better than dense. Readability counts. Special cases aren't special enough to break the rules. Although practicality beats purity. Errors should never pass silently. Unless explicitly silenced. In the face of ambiguity, refuse the temptation to guess. There should be one-- and preferably only one --obvious way to do it. Although that way may not be obvious at first unless you're Dutch. Now is better than never. Although never is often better than *right* now. If the implementation is hard to explain, it's a bad idea. If the implementation is easy to explain, it may be a good idea. Namespaces are one honking great idea -- let's do more of those!
This can be seen by typing import this
inside the Python interpreter.
Upvotes: 8