Search code examples
conventions

What is the proper format when you provide each possible outcome in an if-else chain?


I ask this in terms of Java, but it could apply to other languages that follow a similar compiler pattern as well.

This can apply to any type of method where you provide all possible outcomes, but I will use a boolean method as an example. I have listed 2 ways to code this method:

Example 1:

public boolean example1(final int value1, final int value2) {
    if (value1 == value2) {
        //do something 1
        return true;
    } else if (value1 < value2) {
        //do something 2
        return false;
    } else if (value1 > value2) {
        //do something 3
        return false;
    } //could also be in an "else" block.
    return false; //impossible to reach, but necessary to compile.
}

Positives:

  1. Allows for future viewers of the method to immediately determine exactly what the method does.

Negatives

  1. A bit "hacky", and almost unclean.

Example 2:

public boolean example2(final int value1, final int value2) {
    if (value1 == value2) {
        //do something 1
        return true;
    } else if (value1 < value2) {
        //do something 2
        return false;
    } else { //The same as (value1 > value2)
        //do something 3
        return false;
    }
}

Positives:

  1. Is not immediately clear what the method does. (Although this could be prevented with proper documentation)

Negatives

  1. Not immediately obvious what the method does.
  2. The else, at least to me, might imply that the only important comparisons are the first 2. After all, else means "everything else", which might be misleading and/or confusing.

These methods do the exact same thing. However, I believe that Example 1 is easier to read than Example 2, simply because it explicitly states the last if statement in the if-else chain. At the same time, Example 1 feels extremely, for the lack of a better word, dirty to me.

Is there a proper way to do this, or is it just personal preference?

EDIT: I chose to not include a set of if statements (not in an if-else chain) because I know for a fact that it is not a good way to do this. If someone disagrees with this and has a valid reason for doing so, please post below, and I will modify my question.

EDIT 2: The reason for this opinion is because if "do something" is, for example, 100+ lines of code, not having the if-else chain would reduce readability substantially. In short examples where "do something" is only 1 or several lines of code, then yes, that might be viable. For this reason, I will add it as a third example below (although I still don't think it's very readable, but that's personal preference.)

Example 3:

public boolean example1(final int value1, final int value2) {
    if (value1 == value2) {
        //do something 1
        return true;
    }
    if (value1 < value2) {
        //do something 2
        return false;
    }
    //do something 3
    return false;
}

Positives:

  1. Easier to read (for some people) when "do something" is only several lines of code.
  2. Removes the technically redundant "else" statements.

Negatives

  1. Much more difficult to read in situations where "do something" is hundreds of lines of code.
  2. Having the "do something 3" outside of an if-statement indicates at first glance that the line will always be run, until the return statements are noticed in the previous if-statements.

Example 4 (and my personal favorite):

public boolean example1(final int value1, final int value2) {
    if (value1 == value2) {
        //do something 1
    } else if (value1 < value2) {
        //do something 2
    } else if (value1 > value2) {
        //do something 3
    }
    return value1 == value2;
}

This is great because it takes care of the "do something" first, and then confines the return statement into a separate, clean, easy to read statement, with no "hacky" repercussions.


Solution

  • Personally, I'm a fan of only having one return statement when it makes sense. For your code, I think it does. So, I would use...

    Option 3:

    public boolean example1(final int value1, final int value2) {
        if (value1 == value2) {
            //do something 1
        } else if (value1 < value2) {
            //do something 2
        } else if (value1 > value2) {
            //do something 3
        }
        return value1 == value2;
    }
    

    This particular structure doesn't always make sense, but since you're always returning value1 == value2 anyway, I think it clears things up a bit to only do it once at the end.

    To anyone coming along and glancing at the code, it will be pretty clear what is happening. The method always completes in the same location with the same value, and there is a simple bit of workflow logic before that.