Cleaner if-statements: Indentation hell & failing fast

Let’s have a look at two common issues with if statements: Indentation hell aka too deeply nested if statements and fail fast if statements at the beginning of a certain code block instead of wrapping the whole code block with an if statement.

Indentation hell

If you have a look at the Linux kernel coding style you will find the famous sentence (and I quote) “if you need more than 3 levels of indentation, you’re screwed anyway, and should fix your program”. This was written over twenty years ago in 1996 and can be found in the Documentation section in version 2.0 of the Linux kernel.

The idea was to write code blocks that handle a certain issue. If too many concerns were added to a function instead of accepting a lot of if statements, for loops and so on the function should be divided into different parts in such a way that, again, only one issue is handled per function. Even though this optimum may not be easy all the time at least trying to strive towards this should lead to more readable code.

That being said let’s have a look at a function that converts an external order to an internal data format consisting of Order objects that can hold multiple OrderItem objects. It starts with a for loop that creates the internal order based on the external one. It continues with an if statement, a for loop and a few more if statements that construct the OrderItem objects. Finally the orders are returned.

public List<Order> indentationHell(final List<ExternalOrder> externalOrders) {
	final List<Order> orders = new ArrayList<Order>();

	// 1
	for (ExternalOrder externalOrder : externalOrders) {
		final Order order = new Order();
		order.setOrderId(externalOrder.getExternalOrderId());
		order.setBuyerName(externalOrder.getBuyerName());

		// 2
		if (externalOrder.getJsonOrderItems() != null && externalOrder.getJsonOrderItems().isEmpty() == false) {
			// 3
			for (final String jsonOrderItem : externalOrder.getJsonOrderItems()) {
				final OrderItem orderItem = new OrderItem();

				// 4
				if (jsonOrderItem.isEmpty() == false) {
					orderItem.setItemName("set order item name here...");
					orderItem.setQuantity(1); // Set correct quantity here...

					// 5
					if (jsonOrderItem.contains("SHIPPING") == false && jsonOrderItem.contains("GIFT_WRAP") == false) {
						// 6
						BigDecimal currentPrice = new BigDecimal("42"); // Set correct price here...
						if (jsonOrderItem.contains("###-SPECIAL_PRICE-###")) {
							currentPrice = currentPrice.multiply(BigDecimal.TEN);
						}
						orderItem.setPrice(currentPrice);
					}
				}

				order.getOrderItems().add(orderItem);
			}
		}
	}

	return orders;
}

A possible improvement would of course be to split the different tasks and create different methods for it, namely one to create the internal orders and another one to create order items. Without any further changes the nesting would already be reduced and the code would be drastically more readable in my opinion.

public List<Order> indentationHell(final List<ExternalOrder> externalOrders) {
	// 1
	return externalOrders	.stream()
				.map(this::mapOrder)
				.collect(Collectors.toList());
}

private Order mapOrder(final ExternalOrder externalOrder) {
	final Order order = new Order();
	order.setOrderId(externalOrder.getExternalOrderId());
	order.setBuyerName(externalOrder.getBuyerName());
	// 2
	if (externalOrder.getJsonOrderItems() != null && externalOrder.getJsonOrderItems().isEmpty() == false) {
		// 3
		order.setOrderItems(externalOrder.getJsonOrderItems()
						 .stream()
						 .map(this::mapOrderItem)
						 .collect(Collectors.toList()));
	}

	return order;
}

private OrderItem mapOrderItem(final String jsonOrderItem) {
	final OrderItem orderItem = new OrderItem();

	// 4
	if (jsonOrderItem.isEmpty() == false) {
		orderItem.setItemName("set order item name here...");
		orderItem.setQuantity(1); // Set correct quantity here...

		// 5
		if (jsonOrderItem.contains("SHIPPING") == false && jsonOrderItem.contains("GIFT_WRAP") == false) {
			// 6
			BigDecimal currentPrice = new BigDecimal("42"); // Set correct price here...
			if (jsonOrderItem.contains("###-SPECIAL_PRICE-###")) {
				currentPrice = currentPrice.multiply(BigDecimal.TEN);
			}
			orderItem.setPrice(currentPrice);
		}
	}

	return orderItem;
}

There are a lot of ways to reduce nesting just by changing if statements. I would like to show you an easy and very effective solution by using the idea of “failing fast” in your code.

Failing fast

If you see that a certain code block is only executed when a special condition is met and there is no else statement the chances are high that you can convert that to a fast failing if statement by inverting the logical condition. These if statements are also called preconditions or guard clauses.

Let’s have a look at the following example.

public String failFast_1(final String str) {
  if (str != null) {
    if (str.trim().isEmpty() == false) {
      if (str.startsWith("START_")) {
        return "START";
      } else {
        return "END";
      }
    }
  }
  return "UNKNOWN";
}

The given String should be analyzed and if it starts with “START_” the method should return “START” and “END” otherwise. In case the variable is not set or the string is empty we return “UNKNOWN”.

Instead of nesting the statements this way we can refactor the precondition “string parameter must not be null or empty” to a guard clause using the StringUtils helper method isBlank from Apache Commons Lang3. This way the other conditions in the method become much easier to read too since we can remove the unnecessary else in case we want to return the String “END”.

public String failFast_2(final String str) {
  if (StringUtils.isBlank(str))
    return "UNKNOWN";

  if (str.startsWith("START_"))
    return "START";

  return "END";
}

Even if you are refactoring seemingly trivial code like this try to take the time to write a unit test just to make sure that your changes did not alter the behavior of the method. More often than not you will forget to consider a special edge case that has even more special side effects.