One of the most common things I see around that I think makes code more difficult to read is the overuse of "if" statements. It’s one of the first programming tools we learn, and it is usually when we learn that the computer can do pretty much anything we like, “if” we use those statements right. Following usually are long ours of printfs and debugging and trying to figure out why the program isn’t getting into that third nested "if" as we're sure it would do! That’s how many of us have approached programming, and that’s what many of us have grown used to.
As I studied more about code design and best practices I began noticing how using lots of if statements could be an anti-pattern, making the code worse to read, debug and maintain. And so out I went to find alternative patterns to those if statements, and I think the patterns I've found have improved my code’s readability and maintainability.
Of course there’s no silver bullet and any pattern should be used where it makes sense. I think it’s important for us developers to have options when attempting to write good quality code.
To illustrate this point I’ll share two simple examples, and I hope to hear your thoughts on the matter in the comments.
First let's see the example where we have to handle an incoming message from outside our application boundaries, and the object contains a string property specifying a type. It might be an error type from a mass email marketing, and we would have to translate that to our own domain’s error type.
Usually I see that implemented like this:
private ErrorType translateErrorType(String errorString) {
if ("Undetermined".equals(errorString)) {
return UNKNOWN_ERROR;
} else if ("NoEmail".equals(errorString)) {
return INVALID_RECIPIENT;
} else if ("MessageTooLarge".equals(errorString)) {
return INVALID_CONTENT;
} else if ("ContentRejected".equals(errorString)) {
return INVALID_CONTENT;
} else if ("AttachmentRejected".equals(errorString)) {
return INVALID_CONTENT;
// } else if (...)
} else {
throw new IllegalArgumentException("Error type not supported: " + errorTypeString);
}
}
You see where this is going, right? It could be a switch statement, and it would be just as cluttered, with much more language specific words than actual business domain language.
There are a number of approaches to getting rid of this kind of “if” entanglement, but I guess the simplest one is the use of a map.
public class ErrorTypeTranslator {
private final static Map<String, ErrorType> errorTypeMap;
static {
errorTypeMap = Map.of(
"Undetermined", UNKNOWN_ERROR,
"NoEmail", INVALID_RECIPIENT,
"MessageTooLarge", INVALID_CONTENT,
"ContentRejected", INVALID_CONTENT,
"AttachmentRejected", INVALID_CONTENT
// (…)
);
}
public ErrorType translateErrorType(String errorTypeString) {
return requireNonNull(errorTypeMap.get(errorTypeString),
() -> "Error type not supported: " + errorTypeString
}
}
See the difference? Now the business logic is front and center, and any developer approaching this class should be able to very easily understand what it does and change it should it be needed. Lots of language specific words and symbols have disappeared making way for a much nicer, cleaner and less error prone code.
This kind of pattern is also very good for simple factories, where you can have a map of singletons, or even suppliers as values. For example, let’s say you have to return a handler bean based on an enum retrieved from the database.
What I usually see is something like:
public class IntegrationHandlerFactory {
private final EmailIntegrationHandler emailHandler;
private final SMSIntegrationHandler smsHandler;
private final PushIntegrationHandler pushHandler;
public IntegrationHandlerFactory(EmailIntegrationHandler emailHandler,
SMSIntegrationHandler smsHandler,
PushIntegrationHandler pushHandler) {
this.emailHandler = emailHandler;
this.smsHandler = smsHandler;
this.pushHandler = pushHandler;
}
public IntegrationHandler getHandlerFor(Integration integration) {
if (EMAIL.equals(integration)) {
return emailHandler;
} else if (SMS.equals(integration)) {
return smsHandler
} else if (PUSH.equals(integration)) {
return pushHandler
} else {
throw new IllegalArgumentException("No handler found for integration: " + integration);
}
}
}
Let´s try using a Map instead:
public class IntegrationHandlerFactory {
private final Map<Integration, IntegrationHandler> handlerMap;
public IntegrationHandlerFactory(EmailIntegrationHandler emailHandler,
SMSIntegrationHandler smsHandler,
PushIntegrationHandler pushHandler) {
handlerMap = Map.of(
EMAIL, emailHandler,
SMS, smsHandler,
PUSH, pushHandler
);
}
public IntegrationHandler getHandlerFor(Integration integration) {
return requireNonNull(handlerMap.get(integration),
() -> "No handler found for integration: " + integration);
}
}
Much neater, isn’t it? Once again a very simple design that gets rid of many if / else if statements and makes it very easy to add new options.
There are many other patterns such as this, and I might do a post about those soon.
So, what do you think about this kind of patterns? Have you ever used a pattern like this? Are you more comfortable dealing with "if" statements?
Please let me know in the comments, I'd love to hear your feedback!
I made a post about another pattern that helps avoiding repetitive ifs, want to check it out?
Latest comments (122)
Hi Tomaz,
Does it impact performance when using map ?
Thank You for your post. It's a diffucult argument, also because you need a balance between different aspects such as performance, design, readability, ... and different answers come from different paradigms. Look at this: github.com/stefanofago73/IOPvsOOP/... I think of it like a starting point: in the future I desire to build a larger store of anti-if/better-if design, so... thank you for your posts, keep going and good work!
As a junior learning to read code written by other devs, I loved reading this. If statements seem to have their place, but when those conditional lists get really long they become much more difficult to debug/fix.
The usage of a map fits the mapping of handlers here.
But in general you don't want to over-engineer 15-line piece of code of if just for the beauty of it, especially if you have business logic in it.
Also, it is not canonical in debugging aspect : each time they need to cold read that code, people will feel the need to check Map.get(...) behavior in docs for non-existent cases, thus adding technical debt for no practical benefits.
Hey Tomaz
A very nice article. You described some effects I figuerd out after I started TDD.
In many cases, after I performed a code coverage I had discovered many if statements simply a good initial idea but completly unneccesary.
You solution using a Map is nice. I had chosen for me Enums to allow a reuse.
regrads ED
for cases described above I don't even like using hashmaps, I just use enums, which I think provide even better compile time safety and are way more declarative and don't enable putting in as parameter any unexpected value. Then there's no need for the IllegalStateException at the and, as an else branch, so the code is even shorter. So I like the idea of removing senseless if statements as in those examples, but using map as an alternative, I find that rather dubious solution, that is prone to having that IllegalStateException happen in production. I don't like those kind of land mines silently waiting to explode.
Hi Lukas! I think the situation where an exception would be thrown is the situation in which the string received from the outer application doesn’t match the ones in the map, and with an enum it would throw an exception anyway...
I used to like using enums more for this kind of translation, but I think sometimes it leads to unnecessary coupling. But I could in fact have created a new enum just for this translation, that’s one way to go!
Thanks for your feedback!
This pattern looks like a very rudimentary form of pattern matching in FP-oriented languages. I use it a lot for similar reasons you mentioned.
In Scala, for example:
There's a lot more, probably warrants its own blog post 😁
Hi Herdy, you're not the first one to mention Scala as a comparison, but I haven't really tried it yet. I'll be looking forward to this post then!
I've just written a post on a different pattern, if you'd like to take a look and give your two (or more) cents I'd really appreciate!
dev.to/tomazlemos/keeping-your-cod...
Thanks for your feedback!
I like the approach of coalescing a huge nest of
ifstatements into a map lookup - it makes the code easier to read and understand! Like anything, though, you can overdo it - it doesn't help much for a 2-choice branch, and may use more resources than a simple if/else would have. Thanks for sharing!I decided to rewrite that post based on your and other fellow DEV's feedback, if you'd care to read the new version I'd really appreciate!
dev.to/tomazlemos/let-s-talk-trade...
Done. This is why DEV is great - we help each other get better!
Thanks a lot Eric!
Hi Eric! I’m glad you liked it, I like it a lot too! I’m in need of feedback for this other pattern, if you could take a look I’d really appreciate hearing your thoughts 😃
dev.to/tomazlemos/implementing-a-s...
Thanks for your kind reply, and Happy New Year!
Done, glad to know it's useful.
Super useful Eric, thank you very much!
The original switch statement was logically complete, the map example introduces the possibility of application exceptions. So it may be more readable, but it's more likely there is a bug, that isn't a win in my book
I work in Javascript but sometimes I see Java code at work. I can see what you're saying but I wonder if you think the "if" ridden examples could be included in unit-test code?
Hi Jen,
I don’t really understand what you mean by including in unit-test code, could you clarify? I should have included test examples, but this more declarative approach is not any different to unit test then the if-version.
Thanks for your reply!
Great pattern. But this pattern won't suit if we need a case insensitive search. Or am I missing something?
Hi Sathesh! You could use map.get(key.toLowercase()), and have all your keys lowercased as well, that way you’d have a case insensitive search.
I’ve shared another pattern in a new post, if you’d like to take a look: dev.to/tomazlemos/implementing-a-s...
Thanks for your feedback!
Hi Tomaz
The most that I like from your article is the philosophical discussion that started in the comments on how to code :)
It looks like that there will never be a final answer .
Anyway....thank you very much for the article .
Adi
Makes me sad when that gets called "clever". "Mapping A B C to X Y Z by using a Map object? Too clever!!" Good grief. I'd fail the else if chain in code review and suggest the switch statement. Map is ok too.
But the article title had me anticipating ternary expressions and array.filter(lambda) as well.
But even with, I don't think of if statements as unclean, just that there's more succinct ways of doing some of the more common things that they're used for.
That's a very Perlish way of handling this logic. That should be taken as a compliment btw.
Hi Nathan, if that's a compliment I do thank you, but I really don't know what that's supposed to mean! 😄
The comments seem to be split on whether removing the "if" statements is an improvement or not. I expected to be on the side of removing "ifs" (it multiplies the number of code branches that have to be treated for coverage), but these examples were completely underwhelming and I find myself on the other side, in favor of the "ifs".
For context, I have 35 years of programming experience that included several large projects in more than a dozen languages.
Nested "ifs" without a clear pattern are definitely bad. As are "else if" in some places and "if" in others (breaking the chain) or some clauses that "break"/"return" while others don't (breaking the control flow pattern). You can spaghettify code that way, though not nearly as bad as "goto".
However, a uniform, flat "else if" chain with simple predicates and a logical order is quite a good pattern, and some of your examples satisfy that pattern. I started to use this pattern more after a stint of Scala programming: it was the closest translation of the "match" syntax used in that language, to good effect. (Of course, you can't translate the pattern matching/unpacking to most languages, but I came to believe that that isn't the most important part: the clear, table-like layout of predicates is.)
Arguments about imperative vs declarative are off-base. "Declarative" is a human concept, not a mechanical one: code is declarative if you use it declaratively. The language might not enforce it, but you can enforce it manually or tweak a linter to check it, if you really need that. To write a declarative block, just don't use state-changing or otherwise order-dependent statements in your "else if" predicates (i.e. only "const" methods). The map is also declarative, but if you ever need to add a non-trivial predicate (which can still be completely declarative!), you'll be forced to turn everything back into "ifs" anyway, or add something worse to handle the pattern-breaking case.
Even more importantly, in the solution using a "map", the "map" is far from the place where it is used, meaning the person reading the code has to go look it up, if that's even possible (i.e. if it's dynamic, not static/const). Unless, of course, we're taking about a case in which we really do want to separate the mapping from the code, because what it represents is more data-like than instruction-like, which is a case that takes some judgement to recognize.
Arguments against "if" because it's one of the first constructs programmers learn are way off-base. "If" is in every language and it's taught early because it's one of the most versatile, simplest language constructs in existence. The fact that everybody knows it is an argument for it, not against it. New programmers will understand the codebase more quickly, which is the whole point, after all.
Are you really "cleaning up" your codebase if it's harder for novice programmers to read? Clarity is vital, but this simple rule of reducing "ifs" (in the way you suggest) seems to be going against clarity.
Hi Jim! Thanks a lot for your feedback, and for contributing with your many decades of experience!
Sorry if the examples are underwhelming, I tried to start with the simplest ones, but perhaps you're going to like the next ones better.
You made some very interesting points.
Some fellow developers commented here about discipline, and I do think that if you can have disciplined code, everything can be used successfully. But in my experience whenever a junior, or even more experienced programmers has to change something in those if/else chains, they will just add a second or third nested if and go about their way. Over time, that gets pretty much unmaintainable, and there comes a time when one can only really understand the code by debugging it, which for me is the very definition of bad code.
I don't agree with your point that if a condition changes we would have to get back to if else statements. You could for example use a map of and use one or two private methods so you could keep it just as declarative. Or just have one if or two to handle the different cases, it's hard to reason about without a real example. But if the conditions are fairly complex, using a Map is surely not your goto choice.
I never said if's should be avoided "because" they are the first thing we learn. I said I think the reason people are so attached to it, and to the imperative style of coding that it more often than not implies, is that it's the first thing that makes us feel "in charge", and some developers never question if there are other ways of programming, which is not that good for us as a community, and that's one point this post seeks to be useful in.
I really don't buy this argument that the Map version is more difficult for anyone to understand, whether it's a novice developer or an experienced one. You may have to think a little the very first time you encounter this pattern, but even a trainee can figure it out rather quickly, or am I wrong? It's just a map.get(), not rocket science...
And I have to say that the point most of the comments are making, about ifs or not ifs, is not really the point of the patterns, and that's my bad for the way I have presented them. My point is about making business rules as clear as possible, and I do think these if patterns are a roadblock to that.
Thanks again for your input, it's been a very interesting and useful discussion!
For clarity, I don't think the "map" version is conceptually hard, but the requirements of the language can force the map definition to be far from its evaluation. Distance is a big problem. Distant relationships in the codebase don't make the concept hard, but it's a speedbump slowing down the code-reader's understanding of what it does.
On another point, the change that can force a reversion to "else if" chains would have to be one that introduces (for the first time) the need to evaluate a function to make a decision. If it's always a straight mapping from values to values, a map would always work. It's that unexpected increase in complexity that doesn't sound like a big deal when setting requirements (and shouldn't be a big deal) that would be an incremental change in "else if" but completely break the map solution (unless you decide to put lambda expressions in the map or introduce a convention that makes it call some class's methods, but that completely undermines the simplicity of the map, which was its selling point).
Complicated, nested "ifs" are a problem, but a local one. If I encounter something like that, I might take an hour to detangle it (particularly if it's touching something else I intend to change). In other words, "if" messes do not involve distant relationships and can therefore be more confidently fixed (especially if there are tests).
I'm speaking up because it looks like this advice would lead intermediate programmers to turn fixable messes into more difficult, functionality-restricting, "clever" ones.
Well, in my experience so far I’ve never had the case where the map’s initialization has gotten so far away from the logic that uses it, because it’s supposed to be just a factory, or a translator. If your logic grows that much, perhaps you’re adding responsibilities to the wrong class? I don’t think these classes should have more than around 50 lines of code so that’s really a non-issue for me.
I have been using these patterns for some time now and have yet to see this “doom’s day” scenario you talk about, but yet I have reaped the benefits of having the business rules so clear. I also find these patterns very, very easy to debug, because you really don’t have room for bugs... Just look at the map and there’s all you need to know.
I have used maps with lambdas as well, with predicates as keys to iterate and filter out the true value. Just put the lambda in a private method with a proper name and it’s just as clear to read as the pure value ones. Not rocket science either and results in very readable business rules.
I disagree that if statements don’t involve distant relationships, or perhaps I didn’t understand what you mean by that. If you have ever encountered a 200 lines (or much more) long method, as I’m sure you have, you know that if based solutions can have really long distances for instance between a variable declaration and it’s uses, with a great chance of occurring a mutation in it’s value along the way.
And we can’t really argue the cons without accounting for the pros... How much time have you spent reading if-infested code just to try to understand what that code does? I think being able to easily understand the business rules is a huge selling point, and I think that’s what these patterns provide. And I do think it’s worth some extra implementation time to have the code more readable and less error-prone. As always, it’s a tradeoff.
As for the intermediate developers, I’m pretty sure they can handle these maps. It’s a different way to handle very common problems, which brings new challenges and new possibilities.
Thank you very much for sharing, I’ve been learning a lot from these feedbacks!
Some comments may only be visible to logged-in visitors. Sign in to view all comments.