Best practice for redundant conditions in if-elif-else statements

  softwareengineering

What is considered better practice?

Case 1:

if n == 0:
    doThis()
elif n < 0:
    doThat()
elif n > 0:
    doSomethingElse()

Case 2:

if n == 0:
    doThis()
elif n < 0:
    doThat()
else:
    doSomethingElse()

The first case is more explicit and (arguably) clearer.

The second case doesn’t handle the incorrect case when n is not an integer. Pass in a non-integer object (one that implements __lt__ and __eq__) and it returns without throwing error. This isn’t expected behavior. If this was written in a statically-typed language, this wouldn’t be a problem. But this is Python.

The second case is, however, potentially quicker, because it avoids an extra conditional expression evaluation (i.e., the n > 0 statement).

9

Short answer

I would argue that there is no singular correct answer to this question.

It depends on what you want to have happen when one of the above cases changes. Are you assuming that the last case catches everything that’s left? While the previous question may seem an obvious rephrasing from asking if you want it to work the way an else case works, sometimes the simplest questions are the ones that get you to the rightest answers.

Much longer answer

When you ask what is better, “better” here includes checking which is best logically, readably, speedily, and maintanably.

Logically, they’re equivalent.
Edit: I was not aware of NaN in Python being an option here. The two options are only logically equivalent if they both cover every possible value, but NaN is not being covered for here. However, if you could somehow guarantee that NaN would not be a possible value here (e.g. this already being established in an earlier part of the code), then they would be logically equivalent.

Readably, the elif option is the most informative as it describes the case. However, I could make the point that a code comment would achieve the same purpose. So while the elif option slightly favors readability, if we find a reason to favor the else option for other reasons, we can mitigate this by adding a comment to the else option.

Speedily, the else variant is better due to having one less redundant consideration. That being said, what I just said only applies if this last condition fully covers every possible scenario that is left (i.e. after all the other conditions have turned up false already), and if you care about this level of performance. Generally speaking there is not such a performance constraint, and the question being asked here is more one of readability and maintenance (as so many good practice questions are).

So far, elif favors readability (but only by a tiny margin) and else favors performance (but only in relatively rare scenarios). I’m going to call this mostly a tie.

Maintainably, this is where the context matters. You used an example of a complete set (all integer numbers), and you picked cases that together form the whole set. There is no reasonable expectation that new numbers will be invented. Therefore, the last condition of the elif option is truly redundant.

However, you’ve unknowingly cherrypicked an example that introduces a constraint that doesn’t always apply. I suspect you were asking a much broader question, so I’m going to use a different example that addresses a much broader set of use cases.

Let’s say we have a company with five employees. Two of the staff are managers. The other three are developers. In this company, managers earn $150k, developers earn $100k, and no one has (or will ever have) personally negotiated salaries.

You can now rephrase your question to this example, which asks whether this …

if(employee.Role == "Manager")
    salary = 150000;
else
    salary = 100000;

… or this …

if(employee.Role == "Manager")
    salary = 150000;
else if(employee.Role == "Developer")
    salary = 100000;

… is better. You may have already noticed what’s different about this example compared to yours: this is not a complete dataset. Over time, it’s possible that you might hire an accountant. What should their salary be?

I very intentionally kept the salary rules vague, leaving it unspoken how the rule is designed. There are two main possibilities here:

  1. Everyone earns the same salary, but managers get a $50k bonus.
  2. Each role has a specifically agreed upon salary.

In the first case, an accountant inherently earns the same as a developer. Therefore, the else case is the better option, because it accurately separates the managers from “everyone else” (notice the “else” there).

In the second case an accountant’s salary and a developer’s salary have no relation to each other, and therefore the logic should not group them together. Therefore, the elif case is the better options, because it ensures that the salary amounts are correctly tied to the role that they belong to.

The question here is what happens when you forget to update this logic. With the else option, accountants will immediately earn the same as developers, which matches the first case listed here. With the elif option, accountants will not be given a salary until someone has explicitly decided what it should be, which matches the second case listed here.


Conclusion

Because I can’t resist the opportunity here:

if(performance_matters_a_lot)
    return "else";
else if(current_logic_represents_final_and_complete_set)
    return "else";
else
    return "elif";

Yes, I did doublecheck that using else was consistent with the information in this answer.

8

Even Case 1 can have unexpected behaviour. The value of n that worries me is NaN.

n = float('nan')
if n == 0:
    doThis()  # Not triggered
elif n < 0:
    doThat()  # Not triggered
elif n > 0:
    doSomethingElse()  # Not triggered

This would be a problem if the rest of the program assumes that one of these has been run. To avoid this you could raise an exception from a final else:

if n == 0:
    doThis()
elif n < 0:
    doThat()
elif n > 0:
    doSomethingElse()
else:
    raise Exception

10

In the specific case of n > 0 vs n < 0 vs n == 0, I think it’s best to write the == case last and use else for it, but with a comment, like this:

if n > 0:
    ...
elif n < 0:
    ...
else: # n == 0
    ...

It is easier for a human reader to see at a glance that the comment is true if the cases are listed in this order, than if n == 0 is one of the first two cases.

Your concerns about supplying an argument of the wrong type to this function would be better addressed with type annotations.

3

Your own primary objection to your version 2 (with the unconditional else) seems to be that it acts wrongly when the type of the variable is not integer.

I’d argue that your first version is wrong as well. If n is not an integer, it will not enter any of the three cases and do nothing. This would not be the expected behaviour – I would want to have some indication that something went very wrong. Just silently swallowing the error condition by doing nothing is the same (in my opinion) than erroneously doing something. Both can be incredibly frustrating to debug, and can cause untold harm and suffering to your data.

So. If your data can be wrong in some ways (be it type, domain, etc.) then you should do error checking before using the data. This is obviously mandatory when using data that comes from the outside.

For a method like this, modern Python does offer type checking of arguments (arguably only in IDEs etc, not enforced).

To answer your original question; my personal preference is the style with a dangling unconditional else. And if I’m in doubt, then that case will be pure error handling (i.e., raising an exception if the language supports it).

I.e.:

if (someChecksFailed(n)):
  raise ValueError(f("Invalid argument: {n}")

if a(n):
  doA()
elif b(n):
  doB()
else:
  raise ValueError(f"Huh? Unexpected argument: {n}")

Or, if I’m very sure that nothing can really go wrong after my checks:

if (someChecksFailed(n)):
  raise ValueError(f("Invalid argument: {n}")

if a(n):
  doA()
elif b(n):
  doB()
else:
   doC()      

Finally, if “doing nothing” is a perfectly valid possibility, you can make it explicit:

if (someChecksFailed(n)):
  raise ValueError(f("Invalid argument: {n}")

if a(n):
  doA()
elif b(n):
  doB()
else:
   None      

Performance differences should be so irrelevant here that I would never take them into consideration at this point – if and when it turns out that that spot is for some reason performance sensitive then it’s time to optimize, but this kind of construct should first and foremost be correct and easy to maintain.

5

The real problem is: You are sure there are the three cases n>0, n=0 and n<0, but people who are sure they are right are often wrong. And you would want to express that.

Swift has a switch statement where the compiler requires a guarantee that every case is handled. In your case, you write your three cases. The compiler checks that everything is covered. If not then you must add another case handling the missing values, or a “default” statement. If everything is covered, “default” is not allowed. If the compiler cannot check it, it’s not allowed.

So you would write down your three cases, and everything is clear. Two cases plus default means there might be cases that you missed. Three cases + default is not allowed.

(In another post: What if I change “n > 0” to “n > 1”? The compiler would figure out that not everything is covered and force you to fix it somehow. If you wrote two cases + default then the default would now unexpectedly include the “n = 1” case.

An easy fix in any language would be instead of a final else or else if have a statement “remaining n > 0” which implies an assert that everything is covered, with the compiler optimising things.

As you mentioned python is dynamically typed, hence either ensure using numbers or use elif n > 0:.

(Could even apply some comparison three valued type. Like signum. Or an else exception.)

But you favor speed. The overhead may exists, may be optimized away (in python?), but is minor.

Better optimize by probable number frequency (most frequent first):

if n > 0:
    ...
elif n < 0:
    ...
elif n == 0:
    ...

For a larger number of cases, nest if statements.

LEAVE A COMMENT