To main content

Why you should avoid if-else in your code

Published by Benjamin Marwell on

if/else statements are one of the most used statements of almost every programming language. Whenever you create new source files, a few methods you write will always contain an if-else statement at some point. I tend to refactor these statements out, so that I have almost no else statement in my code.

Read on why I chose to avoid else most of the time and how I apply the rules on an example on a random method taken from Objectos.

Why avoiding else is good - rationale

All the following statements can be disproved easily. They are mostly a design choice you have to make (or not) at some point.

You do not need else

This might sound silly at first, because this statement could be true for so many other statements: Loops could be replaced by goto statements, but you have probably chosen to avoid goto instead. That said, it is just one more »clean code« rule.

Why does it matter?
Look at the alternatives and what will happen if you follow this rule.

Use guard clauses

If you are coding for a while now, you should have stumbled about the idea of using Guards. In short: Put your exceptional cases at the top of your methods and return early. This will allow your main logic flow after all the guard clauses, so you do not clutter your code with unnecessary if-statements.

One might say a disadvantage of guard clauses is the fact that you use multiple return statements now. However, considering the alternative is using multiple, mutable state variables within your method, I consider multiple return statements a much lesser evil.

Replace else with methods

The second rule to follow is replacing variable assignments inside if-else statements with an extracted method.

Applying this rule has multiple advantages:

  1. All variables can be (implicitly) final and unmodifiable.
    This is also beneficial for the runtime performance of your code, as the compiler can apply more optimizations.

  2. You can apply the guard clauses in your new method.
    Now your extracted method will also be split into a »special« logic and a »main« logic.

  3. The extracted method has a name.
    Be sure to give the method a good name. As it now has a new, single responsibility (i.e. returning a value for a new final variable of another method), it should be easy for you to find a meaningful name for your new method.

Have your main logic non-indented by avoiding else

If you followed the two rules above, you should now see an important effect: All your »main logic« is at the end of the method and on the main indentation level. This makes it easier to follow and to read. It also allows easier refactoring if at any point in the future you decide to extract more methods or to implement new logic.

No-else-example: Applying the rules

For this example, I took a random method of Macrio Endo‘s Objectos.

The original code

This is the original code from InternalApi.java. Interestingly, this method declares two non-final variables and uses a few if-else-if statements.

InternalApi#elemItem(Object)
class InternalApi {
  private void elemItem(Object obj) {
    int offset;
    int kind;

    if (obj instanceof JavaTemplate._Item item) {
      offset = 0;

      kind = LOCAL;
    } else if (
      obj == _Ext.INSTANCE || obj instanceof External || obj instanceof String) {
      offset = 1;

      kind = EXT;
    } else if (obj == _Include.INSTANCE) {
      offset = 2;

      kind = LAMBDA;
    } else {
      throw new UnsupportedOperationException(
        "Implement me :: obj=" + obj);
    }

    int index = stackPeek(offset);

    index = levelSearch(index, kind);

    if (kind != LAMBDA) {
      int levelValue = levelGet(index);

      int proto = protoGet(levelValue++);

      protoAdd(proto, levelValue);
    } else {
      elemCntx0lambda(index);
    }

    stackset(offset, index);
  }
}

Refactor out the if-else-block.

The first step is to extract the if-else block. You can use your IDE. IntelliJ will in fact do what I would have done. Since we have two variables set, we need to define a new Tuple-like class. Luckily we are on Java 17 and can use a local private record class for this.

InternalApi#elemItem(Object) after applying the first refactoring
class InternalApi {
  private record OffsetKindResult(int offset, int kind) {}

  private void elemItem(Object obj) {
    final var offsetKind = getOffsetKind(obj);

    int index = stackPeek(offsetKind.offset());

    index = levelSearch(index, offsetKind.kind());

    if (offsetKind.kind() != LAMBDA) {
      int levelValue = levelGet(index);

      int proto = protoGet(levelValue++);

      protoAdd(proto, levelValue);
    } else {
      elemCntx0lambda(index);
    }

    stackset(offsetKind.offset(), index);
  }

  private static OffsetKindResult getOffsetKind(Object obj) {
    int offset;
    int kind;

    if (obj instanceof _Item item) {
      offset = 0;

      kind = LOCAL;
    } else if (
      obj == _Ext.INSTANCE || obj instanceof External || obj instanceof String) {
      offset = 1;

      kind = EXT;
    } else if (obj == _Include.INSTANCE) {
      offset = 2;

      kind = LAMBDA;
    } else {
      throw new UnsupportedOperationException(
        "Implement me :: obj=" + obj);
    }

    return new OffsetKindResult(offset, kind);
  }
}

The main logic is clearer already.

Now, let’s replace all the else statements with guard clauses.

Refactoring getOffsetKind

class InternalApi {
  private static OffsetKindResult getOffsetKind(Object obj) {
    if (obj instanceof _Item) {
      return new OffsetKindResult(0, LOCAL);
    }

    if (obj == _Ext.INSTANCE || obj instanceof External || obj instanceof String) {
      return new OffsetKindResult(1, EXT);
    }

    if (obj == _Include.INSTANCE) {
      return new OffsetKindResult(2, LAMBDA);
    }

    throw new UnsupportedOperationException("Implement me :: obj=" + obj);
  }
}

That’s much shorter already! You can already see some benefits: There are only a few cases which actually return a valid result. The most generic case (not an _Item nor an _Include.INSTANCE nor External or alike) will throw an Exception. We also got rid of the local variables.

I (personally) find this much easier on the eyes to read.

Extracting the other if-else

class InternalApi {
  private void elemItem(Object obj) {
    final var offsetKind = getOffsetKind(obj);

    int index = stackPeek(offsetKind.offset());

    index = levelSearch(index, offsetKind.kind());

    modifyLambdaOrProto(offsetKind, index);

    stackset(offsetKind.offset(), index);
  }

  private void modifyLambdaOrProto(OffsetKindResult offsetKind, int index) {
    if (offsetKind.kind() != LAMBDA) {
      int levelValue = levelGet(index);

      int proto = protoGet(levelValue++);

      protoAdd(proto, levelValue);
    } else {
      elemCntx0lambda(index);
    }
  }
}

Now let’s apply our »no-else«-rule. This is a little different from the example above, because I will reverse the if-order here. I want to handle the single-special-case first. Before our refactoring, we check if something is NOT a lambda. The »special lambda logic« follows in the else branch, which is exactly what I want to avoid.

Look how by adding a single return statement and reversing the logic, we can move the main logic (not lambda) into the first level of indentation.

class InternalApi {
  private void modifyLambdaOrProto(OffsetKindResult offsetKind, int index) {
    if (offsetKind.kind() == LAMBDA) {
      // special case: Lambda handling differs from all other kinds.
      elemCntx0lambda(index);
      return;
    }

    int levelValue = levelGet(index);
    int proto = protoGet(levelValue++);
    protoAdd(proto, levelValue);
  }
}

The final code

After refactoring, we have this final code. I added a few comments for clarity. Please note how each method now naturally has one responsibility without having given it much thought. We didn’t even know what the method does.

class InternalApi {
  private record OffsetKindResult(int offset, int kind) {}

  private void elemItem(Object obj) {
    // get offset and kind or throw early.
    final var offsetKind = getOffsetKind(obj);

    int index = stackPeek(offsetKind.offset());
    index = levelSearch(index, offsetKind.kind());

    modifyLambdaOrProto(offsetKind, index);

    stackset(offsetKind.offset(), index);
  }

  private void modifyLambdaOrProto(OffsetKindResult offsetKind, int index) {
    if (offsetKind.kind() == LAMBDA) {
      // handle the LAMBDA special case.
      elemCntx0lambda(index);
      return;
    }

    // generic case
    int levelValue = levelGet(index);
    int proto = protoGet(levelValue++);
    protoAdd(proto, levelValue);
  }

  private static OffsetKindResult getOffsetKind(Object obj) {
    // starting with Java 21, this would be a nice switch-when.
    if (obj instanceof _Item) {
      return new OffsetKindResult(0, LOCAL);
    }

    if (obj == _Ext.INSTANCE || obj instanceof External || obj instanceof String) {
      return new OffsetKindResult(1, EXT);
    }

    if (obj == _Include.INSTANCE) {
      return new OffsetKindResult(2, LAMBDA);
    }

    // generic case: not implemented
    throw new UnsupportedOperationException("Implement me :: obj=" + obj);
  }
}

Testing code without else statements

Now that we have a few methods with a more restricted responsibility, testing just became easier! We can open those methods for testing in case you want to cover all the cases.

Here are some tests I came up with. Please note that they may not be making sense for Objects; This code is only about showing how easy parts of logic are testable now.

public class InternalApiTest {

  @Test
  public void offsetKind_Item_is_0_local() {
    // given
    Object obj = _Item.INSTANCE;

    // when
    OffsetKindResult offsetKind = InternalApi.getOffsetKind(obj);

    // then
    assertEquals(offsetKind.offset(), 0);
    assertEquals(offsetKind.kind(), InternalApi.LOCAL);
  }

  @Test
  public void throws_exception_on_unknown_object_type() {
    // given
    Object obj = new BigDecimal(0);

    // expect
    var unsupportedOperationException = Assert.expectThrows(
        UnsupportedOperationException.class,
        () -> InternalApi.getOffsetKind(obj));
    assertTrue(unsupportedOperationException.getMessage().contains("Implement me :: obj="));
  }

  @Test
  public void modifyLambda_on_lambda() {
    // given
    var api = new InternalApi();
    var offsetKind = new OffsetKindResult(2, InternalApi.LAMBDA);

    // when
    api.modifyLambdaOrProto(offsetKind, 2);

    // then
    assertEquals(api.protoArray[0], 0);
    assertEquals(api.protoArray[1], 0);
    assertEquals(api.protoArray[2], 0);
  }
}

Conclusion

In this blog post we discovered how we can avoid else statements and at the same time gain code clarity and testability. Please note that applying those rules might not always suit your needs. However, I found avoiding the else statement possible and feasible in almost all situations.

Whether to open the extracted methods for testing is your choice. Especially if they become part of an external API. But with Java 9+ and JPMS, you can opt out of making public classes available to others.

Given these choices, I find avoiding else almost always a good idea.