This code is hard to understand. It’s hard to understand because error handling is distant from the error checks (for instance, the check for nil is at the beginning but the error and return are at the end!). It’s hard to understand because the important parts are deeply indented, giving you less headroom. If you want to add additional checks, it’s hard to know where to add them – and you have to touch lots of unrelated lines to change indent level. And there are many exit points scattered throughout. GROSS.

Whenever I see code like this I cringe. When I get the chance, I like to untangle it (or even catch it in code review). It’s soothing, simple work. To be sure, the functionality of the code is fine – it’s purely how it is written that annoys me.

There’s a key thing to be aware of in the structure of this code – it has a bunch of early outs related to error handling. This is a common pattern so it’s worth walking through the cleanup process. Let’s pull the first block out:

It’s a LOT better, but now we have a return that can never be run. Some error handling code is still far from the error detecting code. So still a little messy. Let’s do the same cleanup again on the second block:

See how much cleaner that is? Beyond saving indents, it also exposes the structure of the algorithm a great deal more clearly – check it out:

Check for nil productId; bail if absent.

Log productId if it is present.

Check if we can make payments/IAP is active; bail if not.

Submit the product info request.

Return success!

The code and its “flowchart” now match up nicely, and if you modify one, it’s easy to identify the change in the other. This might seem like a little thing, but I find it shows that the purpose + structure of the function is well set up. And if you can’t write the function without violating this rule, it’s often a very solid clue you need to introduce some more abstraction – tactics such as breaking stuff up into helper methods, reorganizing your data structures a little bit, centralizing lookups/checks, and so on.

Something to keep in mind next time you find yourself hitting tab more than a couple times – flatten your conditionals!

4 thoughts on “Flatten Your Conditionals!”

Just curious, is there a specific reason an if/else if/else wouldn’t be more appropriate?

Another quick rule of thumb I tend to follow, is to put the smallest blocks first in the conditional tree. This keeps things a bit easier to read, and prevents the tree from becoming top heavy, where things at the end may be missed.

Post navigation

Buy My Game!

Solve Your Problems!

I've developed a love for consulting and run The Engine Company. We solve hard problems in AR/VR, embedded systems, internet video, mobile/IoT, cloud/backend, game technology, fractional CTO and a lot more. Visit our site for more info.