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?
Oldest comments (121)
Thanks for the reply Andy!
I think the discussion in this case could be about what's more important for the reader to know, whether it's understanding "what" a piece of code does or "how" it does it.
I tend to prefer the solutions that are about making the business rules as clear as possible, in a declarative style, because I usually want to understand quickly "what" a piece of code does, and then evaluate whether or not I should spend time understanding "how" it does it.
So in this case, I think it's obviously easier to figure out "how" the "if" version works, but I think it does make a lot more obscure "what" it does.
What do you think, do you agree with this "what" vs "how" analogy?
Thanks again!
My feelings towards this topic are ambivalent. When I first had this revelation that
if
statements could be avoided with different techniques (some of them even through polymorphism) I was very sensible when I ever wrote anif
statement again. But let's be honest: How many cases are there which are really improved by avoidingif
s?They are dead simple to write. And simple to debug. Then there is the
switch
statement and there are OTOH languages like python who do not have a switch statement at all - so you are left withif
andelif
else
.Of course writing the
dictionary
/object
-form is pleasing to read because it appeals to you - the reader - that you feel someone has put a bit of thought into that - and what makes us more comfortable than feeling smart when reading "smart code"?The
if
version feels dirty because everybody could have written it.But when or why do we read the code anyways? For fun and profit - sometimes: But most of the times one reads code, because it is somehow broken.
And coming back to the factory example: The question I would have as a "Debugger" is
And I would start to scan the cascade of
if
s as I would thedictionary
approach and I would assume very little difference in the time taken to make the code work again.So on a "poetic level" I would agree that the cleanup is well spent time. OTOH hand it's like cleaning your house: It would be nice if everthing was 100 percent tidy. But most of the time when dealing with unexpected visitors, it is sufficient to keep a tidy facade even though you know there might be corners where it is a bit dusty.
Hi Thomas, thank you very much for sharing your thoughts!
I relate to that "if" sensibility, I have spent over a year not writing ifs, and that's how I came up with these patterns. Nowadays I do write ifs when I think they will lead to simpler solutions. OTOH, in my experience more often than not what started as a simple and innocent if will grow to a bizarre logic with lots of nested ifs and whatnot, and so I think it's a valid approach to enforce some constraints by providing a well defined interface for the developer to work on, so at least the mess is a bit organised.
"The if version feels dirty because everybody could have written it." - I don't really feel that way... I think the if version feels dirty because there are so many unimportant words concealing the important information, which is the business logic the class is implementing.
I think that the cleaner way has a huge benefit of making it as clear as possible what is the correlation between the two values. In the end, I think programming is all about business rules, and the implementation details should be concealed when possible.
In your debugging example, specially in a more complex situation, I think you could spend a lot more time on the if version if your eyes just didn't catch that one word that's out of place in that sea of words. Or at least it will require a lot more attention than the cleaner version.
But of course it's a matter of opinion, I actually had the word "polemic" in my first draft... And my intention here is to get this kind of feedback so we can all keep reasoning about what good code is, so thank you again!
I think we share a lot. Mostly avoidance of nasty convoluted code. That said it follows that when we both would encounter "a more complex situation" we would do our best to deconstruct it into much more digestible parts.
But I do not share your implicit bias that an if-free version were implicitly clean. Especially when I read
Which anticipates which solution is cleaner 😉
Good code is code which does what it should nothing more.
I do think we share a lot, and it seems like this “if” matter is sooner or later part of the journey of developers who want to get better at writing good quality code.
I think that perhaps the main lesson here is that we should be able to choose how we’re going to write a particular piece of code, and this anti-if crusade is a way of discovering patterns that can be really useful, when many developers never question the “if” status quo and hence could have less tools to address good code requirements.
That said, the post is completely biased in that the if-less examples are cleaner, so it should really come at no surprise that I anticipated which solution I think is cleaner...
And last but not least, I do question your last phrase. I think most developers are really focused in making code do what they want, when the focus should be in writing code that tells you “what” it does.
So many times I have wasted precious time deciphering code only to realize it wasn’t the right place to work in.
I don’t really want to understand how the code works, unless I have to. I just want to take a look at it, understand what business rules it addresses, and see whether it’s the right piece of code to work with or not... If it is, then I’ll spend the necessary time understanding “how” it does it.
And I think the dictionary examples do make a better job at communicating the business rules.
I appreciate a lot being able to discuss this topic here, thanks 🙏🏼
Yes. This discussion is good. And I find it is so on different levels. What I find mostly valuable is that it shows to others not being for so long in the industry that we aren't dealing with truths: We do things for reasons; and taking one or the other way has its own right. And that we both agree to disagree helps others to form their opinion on this topic.
But Java has Case Switch... 🤔
Hi Juan, just don't forget those breaks! 😃 I don't like switch statements at all... But it's more a matter of taste. Thanks for your feedback!
Java's new way of switches is a sight to behold. While more verbose, I will probably be using your Map methodology mixed with a new switch come Java 14's release
Didn’t see this new switch yet, I’ll take a look... Thanks for sharing!
I agree with you that overuse of "if/else" code can become hard to follow. Your simple case is a nice simple example how to get rid of "if/else". I've seen some terrible nested "if/else" that could cause one's head to explode.
Context context context context
Exactly what's been ringing in my head reading almost every comment. Either pattern is suitably applicable depending on the context - the particulars of the codebase, project, target runtime environment, etc. I believe most commenters understand this simple truth, but most posts speak only in absolutes. Design patterns shouldn't be applied unilaterally in "real world" dev.
Philosophically... Yes, I believe the abstraction is preferable in most cases, but there are times when imperative code (conditional stmts) is a better solution.
Hi Brian and Amanda, I agree with you that context may change the applicability of one pattern or another. Most people seem to have liked the code examples, but perhaps I could have taken a better approach in presenting them, to generate less of an "us versus them" response. In the end we're all developers trying to write better code! It's been a really helpful and informative discussion though. Thanks for your feedback!
Hi Luke, thanks for your feedback! I've written a blog post about another pattern, if you'd like to take a look I'd really appreciate your feedback on that as well.
Thanks again and happy new year!
I quite enjoyed this article.
While some may consider several years programming at university, then several more afterward, somewhat experienced, I consider myself a beginner. You can always be better.
Full disclosure I would have clung to the familiar if or switch approaches, that is until I read this article. There's always next time.
I like being able to look at my code and appreciate something well-crafted. It's a matter of personal bias, yes, but it's always helped me when reviewing code especially later.
I recently worked on a small project and would have really loved being able to clear up what was a nightmarish daisy chain of if-else statements.
I was able to clean it up quite well and eliminate many errors but I can see where it would have been great to use this approach. It would also make changing the values in a future version a snap.
In other words, thank you for helping a newbie see things in a different way.
Thank you for your feedback kaydubyew, I really appreciate it!
Great mate I've learnt something today. Using map code looks so neater than if statement where we have to repeat it multiple times .
Thanks for your feedback Govind!
Could also use Enum, would save some memory, rehashing time, incase of adding bit of more entries.
I have to say this being "cleaner" isn't necessarily true to me. I think it is more clever for sure but as someone who works professionally on a bug fixing team I would MUCH rather see the ifs in my codebase than someone trying to be clever but then again, people trying to be "clever" or write less lines is why I have the job fixing bugs that I have which I love. I'm not trying to insult your code, I'm simply saying the basics are something everyone understands and I have seen countless times where a refactoring to produce less lines or "simplify" something is the cause of a bug or difficult debugging to find a bug. Ultimately is all personal preference until performance is analyzed but when you work for an enterprise level company with tons of devs cranking out code it is better not to be "clever" and to be consistent and predictable.
Hi brycebba, thanks for replying!
I hear the complaints about it being
“clever” code. Normally clever code is when someone sacrifices readability or comprehension to create an one liner or to use a design pattern just for using it.
First of all, I don’t see where is the complexity of these solutions. They are just different from what we are used to, but it has one very simple line of code (map.get). One can change the requireNonNull to an “if” if he or she is not used to it.
What I don’t see people talking here is about how easy or hard it is understanding the business rules behind the logic, which is my main point.
I think code should be read like a business document, because in the end of the day that’s what we do: we code business rules.
So I really don’t see these patterns as “clever” code, because in my opinion they enhance the business rules readability.
As for the bug fixing, in my experience it’s really easy to debug such patterns because there’s really one place that can have a bug, which is the map, and it’s very easy to notice when something is wrong. Can’t say the same about those lots of ifs.
Cheers!
I love this post. This is an underappreciated application of DRY.
"if" blocks can become magnets for repeated code that often increases unnecessary coupling to APIs---all kinds of APIs. One example is when you have an if-else statement that both contain the same function call but pass in a different parameter. Instead you can use a ternary to make a simple conditional assignment and then a single function call. It's easier to follow the logic, and if you decide to refactor to a pure function, you only need to change one line to a "return" statement.
Thanks for this Tomaz!
These are good examples of swapping out
if
statements with cleaner, safe alternatives.I want to suggest another idea - To ask the question: "Why am I writing this code in the first place?" In other words - Is having my input come in as a string the right thing to do in this part of my code base?
If it's at all possible, having the input typed (as an enum, or better yet as a dedicated class) is so much better. Granted there are times when this is not possible (ex: coming in through serialization or user input), yet when it is possible, relying on the type system is superior to parsing strings even when efficiently done with a
Map
etc.Happy new year,
@urig
Hi Uri!
As I have (perhaps poorly) tried to contextualize, in that case the response comes from an outside system, so I can’t really change it into an object. In fact the point of the class would be to do as you suggest so in the rest of the system I can use my own business’ enum.
If I used that enum to do the translation directly it would be an unnecessary coupling of my business object to the other system’s response.
Maybe a nicer solution would be to create another enum just for translating the string, and have the map correlate the two enums, might be a solution too.
What do you think?
Thanks for replying!
What a brilliant discussion ☺️! I go around a lot of large companies and organisations and work with the developer teams. the one thing that really disappoints and alarms me is how much code is written in such a way that it is not obvious how it does what it does. Yes, of course code should be written to do what is supposed to do and do it well! However remember the phrase 'Old code never dies it just gets maintained over and over....' (or something like that). How many times I meet developers who's job it is to maintain and update code written by some one who has long gone from the organisation! Code HAS to be readable first and foremost, this article helps to understand that.
I also think that using enumeration or possibly inheritance is a great way around problems like this ☺️
Some comments may only be visible to logged-in visitors. Sign in to view all comments.