The roads of Boston are a knot of one-ways, unexpected curves, and no-left-turns. Local lore holds that they were once cow paths, and by the time road planning was needed the city decided to just use what was there. After all, they already connected all the important places.

Sometimes writing code is like leading your cows across undeveloped land. You take the most natural path that gets you to your destination, winding around streams and skirting forests. The path makes sense to you, but when other people come along they get lost! Often, it's worth taking a step back and seeing if you can bridge those streams, and make more efficient and clear paths to your destinations.

I came across a function, written by a friend, which embodies a common example of "cow path coding". It's clear that it made sense to the writer, as they went through each branching case. But as a reader, it's too hard to keep track of the paths and outcomes. Let's take a look and see how to chop down the code and make nice, clear roads to our destination statements.

Many Branches, Few Destinations

The code in question was controlling alerts shown to the user. The software had two types of event, and each type was configurable to have alerts on, silent, or off. If multiple events went off at the same time, the app consolidated them into a single alert (rather than rapid-fire multiple alerts), using the most visible setting of all the alerts in the batch. See if you can read the code below and understand what I mean (I've simplified the original a bit, but the idea is the same):

if (todoEvents.count == 0) {
   if (timerEvents.count == 0) {
       return nil;
   } else {
       if (alertPrefs.timerDisplayType == AlertDisplayType.Normal) {
           return NormalAlertFactory();
       } else if (alertPrefs.timerDisplayType == AlertDisplayType.Silent) {
           return SilentAlertFactory();
       } else {
           return nil;
       }
   }
} else {
   if (alertPrefs.todoDisplayType == AlertDisplayType.Normal) {
       return NormalAlertFactory();
   } else if (alertPrefs.todoDisplayType == AlertDisplayType.Silent) {
       if (timerEvents.count == 0) {
           return SilentAlertFactory();
       } else {
           if (alertPrefs.timerDisplayType == AlertDisplayType.Normal) {
               return NormalAlertFactory();
           } else {
               return SilentAlertFactory();
           }
       }
   } else {
       if (timerEvents.count == 0) {
           return nil;
       } else {
           if (alertPrefs.timerDisplayType == AlertDisplayType.Normal) {
               return NormalAlertFactory();
           } else if (alertPrefs.timerDisplayType == AlertDisplayType.Silent) {
               return SilentAlertFactory();
           } else {
               return nil;
           }
       }
   }
}

The Bigger Picture

This kind of code comes from a procedural mindset that a developer can easily fall into: starting with each condition and branching in their heads, writing down the results as they go.

When I have a situation with multiple branching factors, or any code at all that is fairly complicated-looking, I try to step back and put the desired outcome into words, forgetting about code for a minute. In this case, our desire is to

Use the most visible alert style of any scheduled alert,
or nil if no alerts are scheduled

Now it's clear to see that the user's preference for a type of alert is not relevant unless some alerts of that type are scheduled. It's also clear that we only have three outcomes: a NormalAlertFactory, a SilentAlertFactory, or nil.

Combine and Conquer

We can simplify our branching using Boolean logic, by combining the presence of alerts and the display type of those alerts into a single condition. And since our result depends on two types of alert, we can again combine these statements about each alert set, so that we have a single condition that must be met in order to reach each result.

Boolean haveTodos = todoEvents.count > 0
Boolean haveTimers = timerEvents.count > 0

if (haveTodos && (alertPrefs.todoDisplayType == AlertDisplayType.Normal) ||
   haveTimers && (alertPrefs.timerDisplayType == AlertDisplayType.Normal)) {
   return NormalAlertFactory();
} else if (haveTodos && (alertPrefs.todoDisplayType == AlertDisplayType.Silent) ||
          haveTimers && (alertPrefs.timerDisplayType == AlertDisplayType.Silent)) {
   return SilentAlertFactory();
} else {
   return nil;
}

The refactored version is much shorter and easier to understand. Now it's clear what our goals are: the return statements are obvious, and each can only be reached through a single path. Using newlines to break up if conditions into related groups is a super useful trick for readability.

If It Seems Messy, Rethink It

While destroying if-nests is admirable, the real takeaway here is stepping back and surveying the landscape whenever things get fuzzy. This can be applied to type and function design, as well, and even larger system architecture. Abstraction is frequently a combining force, turning many steps into one, making comprehension easier by decreasing the granularity. Sometimes you may find that combination is the key to simplicity.