user1211030
user1211030

Reputation: 3050

Nested if's - what's more Pythonic?

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

Answers (5)

harshraj22
harshraj22

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

machine yearning
machine yearning

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

DomTomCat
DomTomCat

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

Dilettant
Dilettant

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

Will
Will

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

Related Questions