4

This question already has an answer here:

These types of if-elseif-else blocks appear all over the place, and in no small number (so the less the better). Every time I have to think and decide: Do I want the simpler or the more thorough of the two... and I don't want to think about it anymore.

For example, if I am expecting only one of three integers (0, 1, 2)... don't I want to know (throw or terminate) if I get something else? (In the case of Example #2, the function do_two() could be triggered by any value other than 0 or 1). Obviously, this may depend on if I want my code to break or to continue upon encountering an anomaly, which rigorous testing should detect anyways (but who can ever be sure?). But I am looking for an answer that is appropriate in all, or most, cases. Please explain your answer, if your reasoning differs from my own.

NOTE REGARDING DUPLICATE: I don't think this is a duplicate for two reasons: The answers to the question suggest using switch statements and code abstraction. I don't use switch statements for something this small due to the overhead, nevermind not liking them much at all. Second, if the code can be abstracted, I may still vary well need to run these checks... plus more overhead. My question is aimed at the necessity in covering corner cases using the final else. Most people, in my experience (which is minimal), would jump to using Example #1 if SCRUMing. I think the best point (comment -> answer?) is yes there is a difference and it depends on whether or not I am coding defensively.

Example #1:

if(!is_null(variable) && is_integer(variable))
{
    if(variable == 0)
        do_zero();
    elseif(variable == 1)
        do_one();
    elseif(variable == 2)
        do_two();
    else
        throw_error();
}


Example #2:

if(!is_null(variable) && is_integer(variable))
{
    if(variable == 0)
        do_zero();
    elseif(variable == 1)
        do_one();
    else
        do_two();
}

marked as duplicate by gnat, GlenH7, user40980, Dan Pichelman, 9000 Dec 30 '14 at 16:06

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 2
    Note that these two code samples don't do the same thing. Example #2 will test variable == 1 even if the earlier test succeeds. This may cause different behavior if do_one() modifies variable. Also note that Example #2 will be slower by a likely trivial amount. In any case, to me, this looks like an obvious switch statement and so shouldn't use if/else at all. – Steven Burnap Dec 19 '14 at 21:09
  • 1
    You answered your question: "this may depend on if I want my code to break or to continue upon encountering an anomaly". – Tulains Córdova Dec 19 '14 at 21:21
  • 1
    Sounds like you might want a switch statement – raptortech97 Dec 20 '14 at 4:37
  • @StevenBurnap Yes, my apologies. I meant elseif - edited. – user58446 Dec 20 '14 at 14:43
5

Ask yourself, what is the expected behavior of your method when the variable is outside the range? Should it:

  • Silently discard it and do nothing? Then your second approach is fine.

  • Inform the caller that the variable is wrong? My answer focuses on this case.

Although I would prefer the first approach, it can be improved. The second approach, on the other way, should be avoided, because it makes debugging difficult, since nothing happens when the value is outside the allowed range.

If the variable is outside the range, you may want to know this early in order to terminate (for example by throwing an exception) early too.

For example, this Python code:

def demo(something):
    if something == 0:
        do_zero()
    elif something == 1:
        do_one()
    elif something == 2:
        do_two()
    else:
        raise ValueError("Something is out of range.")

can be rewritten this way:

def demo(something):
    if something < 0 or something > 2:
        raise ValueError("Something is out of range.")

    if something == 0:
        do_zero()
    elif something == 1:
        do_one()
    elif something == 2:
        do_two()

The if block at the beginning of the method is called the guard clause, a refactoring technique used to remove nested conditionals. Here, we don't have nested conditionals, but it is still useful to move arguments validation at the beginning of the method.

Depending on your language, there may be more to do. Most obvious in some languages, such as Python, is the replacement of if/elif/else by a map. The previous method becomes:

def demo(something):
    if something < 0 or something > 2:
        raise ValueError("Something is out of range.")

    # Creating the map between the values of `something` and the corresponding methods to call.
    map = {
        0: do_zero,
        1: do_one,
        2: do_two,
    }

    # Invoking the method.
    map[something]()

From there, you may want to remove the guard clause to obtain a more compact piece of code:

def demo(something):
    map = {
        0: do_zero,
        1: do_one,
        2: do_two,
    }

    if something in map:
        map[something]()
    else:
        raise ValueError("Something is out of range.")

A positive side effect is also the removal of code duplication: you don't have to specify the allowed range separately from the values, which can be the cause of a mistake where you change the list of values but forget to change the corresponding range.


In order to reduce the code even further, you may rely on the framework. In Python, for instance, accessing a value in a dictionary with a key which doesn't exist throws an exception, which means that this piece of code:

def demo(something):
    {
        0: do_zero,
        1: do_one,
        2: do_two,
    }[something]()

does the same thing, except that the type of exception is different (KeyError instead of ValueError). Given that the method is small and it's easy to understand what's going here when receiving the KeyError, this last refactoring is acceptable. On the other hand, if you're dealing with a larger method or the types of exceptions are more distant, avoid relying on the framework and throw the exception explicitly (and preferably early).

  • +1 and ANSWER for: not suggesting this was the same as the suggested link for elegant ways to handle if blocks, improving on my original suggestion with a guard clause, introducing the term guard clause, considering refactoring, and different languages which is something I hadn't cosidered thoroughly. If you have any other recommendations for compiled languages as well as how interpreted languages may be affected, that would be great to read. Your python statement was a good start. Thank you. – user58446 Jan 2 '15 at 10:57
  • nice. One small note; I'd still probably express the map containment as a guard clause with if ! something in map: raise MySpecialError etc. Guard clauses stack up better than if...else'ing – Rob Jan 2 '15 at 14:42
  • 1
    lol and I say that because when I see an if else I find myself holding the "either or" in my head logically. Guard clauses are kind of a full stop ... everything past the guard clause(s) is top-level logic: map[something](). IMHO – Rob Jan 2 '15 at 15:06

Not the answer you're looking for? Browse other questions tagged or ask your own question.