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 (121)
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
if
statements 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!
Some comments may only be visible to logged-in visitors. Sign in to view all comments.