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?
Top comments (122)
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
ifstatements could be avoided with different techniques (some of them even through polymorphism) I was very sensible when I ever wrote anifstatement again. But let's be honest: How many cases are there which are really improved by avoidingifs?They are dead simple to write. And simple to debug. Then there is the
switchstatement and there are OTOH languages like python who do not have a switch statement at all - so you are left withifandelifelse.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
ifversion 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
ifs as I would thedictionaryapproach 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.
I can't believe the comments I'm reading. The 'if' discussion is part of a bigger subject: declarative vs imperative. And there are plenty of writings about this. The general consent is that you use imperative solutions as far away from your domain logic as possible. When you keep abstracting functions, you will end up will some mathematical and computational functions that underneath are maybe using ifs, switches or fors like map does. These functions are then easily packed in a lib of some sort and you just keep using the declarative style in the rest of your code. It's a longer discussion overall, but to see so many people defending the imperative style...makes me sad. I have seen so many bugs and hit so many problems because of the imperative approach used on the wrong levels and I still have to fight for this because people are unwilling to learn and move past the God damn 'if' chapter and approach my code as 'clever'. It is not clever, it is programming and this approach WILL clean your app for 90% of your bugs and WILL make you move faster when changes are required.
Well said, these comments also make me sad. It means so many programmers still don’t get it.
Reducing Cyclomatic Complexity is vital to reduce bugs. Avoiding the use of a hash and rather using ifs is not clever. It’s short sighted.
Hi Roger, 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!
The problem is that people do not work with significantly large and/or complex programs with multiple developers.
The examples here are simple value mappings, but what happens when those if blocks perform logic? Different types of logic blatantly ignoring the single responsibility principle. The more complicated they get the bigger the chance that there are unmatched use cases.
But that's a code smell? Yes, but if statements are like candy to children who just want to plug some code in somewhere to finish a ticket. Code they don't understand and that gets more complicated with each if statement.
Apart from functional declarative solutions there is also the proper use of OOP which is really what design patterns are all about.
All of these require reasoning about code, which apparently is too much too ask.
Agreed. This is going to sound snobbish. Anyone can code; but not everyone can apply coding principles.
Bold statements, but dubious at best. Don't be clever. Use the if statement. Think about all the overhead you introduce and how much more difficult your code is to debug.
Clearly you don’t get the difference between declarative programming and imperative and would rather stay “not clever”. It’s fine, I hire clever declarative programmers over imperative programmers . Clever programmers who write smart code, less code, which leads to less bugs. The reason you prefer using the if statement is because you find it easier to debug, which obviously is very important to you because that’s what you do a lot of... because of your spaghetti if code. Now that’s dubious .
Hmm. Reading this article and reflecting on the times I have encountered the map pattern I initially did not like it. It separates the logic into two different places. That makes it harder to trace through the code and find the right place to modify. You are however right that I have to do it less often using this pattern.
As an electrical engineer, coding is only a small part of what I do. The rest of my day is spent in planning, documentation, testing for regulatory compliance and debugging. Then again, I write firmware for medical equipment. It must be correct or people die.
Imperative style with lots of ifs takes a lot more unit tests to prove it works compared to declarative style. One shouldn’t be debugging, one should be testing and isolating complexity out of the main program. This thread is scary, frankly.
Don’t worry about cyclomatic complexity, just do a lot of debugging to make sure people don’t die? Hrm ...
If it has to work or people die, then you really should scrutinize every if statement carefully and see if you can get rid of it. Many ifs are bugs waiting to be discovered.
One downside to the map version is it's less flexible.
Are you certain you'll never need to enhance one of those conditions?
Should you ever get to the point where you need more than a single, straight comparison for one of those conditions, you'll only end up having to factor the whole thing back to the original if-else ladder.
I think I would need more motivation to favor a map - for example, if you had a kind of registry where new items could be plugged into the map (say, via constructor or method injection) that would be a practical need for a map.
Personally, I tend to favor the simplest possible language constructs over data types or abstractions, until there's a demonstrated need for something more.
Just my perspective. 🙂
Happy New year!
Just for testing the if version would have been way more difficult to write without any added value.
Map is an interface meaning more flexibility. Yes a little more complex.
The map in the example is static but could be a dynamic configuration or comes from anywhere.
Yes it depends on the context. But this pattern is relevant. I like it
I'm not sure what you mean?
The test burden is the same - it has to cover every case.
If the map was somehow open to dependency injection, this would have been a different case, where you could inject a simpler map with mock values, leading to a simpler test case. But as it is now, the map is an implementation detail - your test needs to cover every supported case, so there's no difference in terms of testing. (Your test should not assume implementation details like a map or an if-else ladder, since these details could change; the subject of this post.)
I wasn't trying to say it isn't relevant or I didn't like it. 🙂
I'm just raising awareness of the limitations - when introducing an abstraction (whether it's one you design, or an existing one, in this case a map) there is always a trade-off in terms of flexibility.
Once you adopt an abstraction, you're limited to whatever that abstraction defines as being the responsibility, which means less flexibility.
Unless you have some other definition of flexibility.
But I think I was clear with my example on what I define as flexibility: the freedom to add another condition that doesn't conform to a precise pattern, in this case the map abstraction.
Of course, you could abstract further to cover use-cases with new conditions, but at that point you're taking on more complexity.
So you have to weigh the pros and cons. Introducing an abstraction is always less flexible and can always lead to more complexity - I think that's definitely demonstrated by the example I gave here.
And the author agrees with me:
Again, that doesn't mean this pattern isn't relevant. It doesn't mean I don't like it. It just means you shouldn't make decisions like these without understanding the trade-offs. You should have a practical reason, which, in my opinion, should go beyond implementation details - if you add abstraction, it should be because there's a practical reason to do so.
For example, a need for dependency injection, which would allow different message handlers to be plugged in, would allow testing with mock message handlers, and so on.
There are plenty of valid reasons to choose maps or other abstractions - this wasn't a criticism of the post, just an addendum. 🙂
Hi Rasmus!
One thing I’ve learned from this post and all the discussion around it is that it should have been more about tradeoffs. I wrote a new post, about a different pattern, based more around the tradeoffs, and I think it’s a far better approach.
About the flexibility, one thing that came to mind is that this pattern is just as flexible as a switch statements, isn’t it? If something coded as a switch statement has to embrace a more complex condition you’d have to refactor it anyway. And yet I’ve never seen anyone argue that this should be taken into consideration when using switches.
I’m writing a new post with some more examples of pattens such as this, and in my experience once you have a minimal toolset of these patterns you can change pretty much any repetitive-if situation into a declarative one, without much effort.
As for the valid reasons for using the pattern, my point is that it makes the business rules a lot more readable, which in my experience leads to easier maintenance and less bugs.
I’d have preferred that the discussion gravitated more towards the business rules clarity than to the if or not if part, but again that’s on me for the way I’ve written the post.
I do appreciate your feedback a lot, thanks!
@Rasmus
We may not see the problem with the same lens. I ll just try to make my points clearer. Hope we could agree later.
A map is a contract and practically a function. You give it a key it does some magic and return back to you a value. Your map implementation is up to you. So by providing a specific map and boom you can do stuff of your imagination. This for me is flexibility as you pick your map based on your requirements.
Unit tests burden
With the map/function version, I give a key and I get back a value. I do not need to go for all possible keys for my unit tests. Because what I m interesting in is the contract i.e ability here to translate an error code to something else.
Hi Breton, those are some very good points, thanks!
I’m in need of some feedback on this other pattern, if you care to take a look I’d really like to hear your thoughts:
dev.to/tomazlemos/implementing-a-s...
Happy New Year!
Hi Breton, thanks for your feedback!
Thanks for sharing, I will read it..
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...
I don't agree you would have to refactor it back to sequence of ifs. Instead, you either have just one or 2 ifs before the call, better than 10 ifs! And when you get to the point where you have several ifs between the lookup and the call, it's time to refactor the special cases into the function calls themselves, then you're back to 0 ifs.
In my opinion, now you have the mental burden of both approaches.
Personally, I'd want to choose one.
In my point of view, if you have to have a couple of if-statements before checking the map, that's a sure sign that the map is the wrong abstraction - for example, maybe the type inside the map is the wrong type and you may need to consider adding more abstraction, e.g. a type that covers all your cases; again, being aware of the complexity trade-off.
Maybe it's worth it, maybe it isn't. This is where the job gets hard 😉
Hi Rasmus! Thanks for bringing in your perspective. Your point is a valid one, in that the map version is a lot less flexible than using ifs. But I was reflecting on it and realised that it's exactly that what I like the most about it, and what frightens me more about those uncontrolled ifs.
I think that good code implies in enforcing constraints, and making intentions and interfaces clear, and it's something hard to achieve when solving everything with if statements.
To use your example, if you have lots of chained if statements and the one before the last is a little different due to an enhanced condition, or has a side effect the others don't, it can be really tricky to find it.
When you enforce the constraints, with a Map for example, you're saying that that's all there is about that logic, that's the contract.
Should the requirements change, perhaps that contract isn't useful anymore, and then you should have to design a new one. Or at least you would have to make it clear that that one condition is different than the others.
Coding for me is all about making intentions clear and stating what the business rules are, and I think this approach helps me in achieving it.
Thank you very much again for your perspective and a happy new year to you as well!
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 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!
Thanks for this Tomaz!
These are good examples of swapping out
ifstatements 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
Mapetc.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!
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!
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!
So which approach is faster? Did anyone test that? It's just another aspect to look at it, which might be relevant in some cases. I used write assembly and when I read code like this I compile it to machine code in my brain. One of greatest performance tweaks can be done with ugly ifs. End user will thank you. Next person taking over your code - not so much
I ran the tests out of curiosity using Java 11 and jmh looking at throughput...
Three implementation,
if else if...,map(hashmap) and aswitch.With one notable exception, the results were as I had expected:
if else if...approach became slower for matches against the "later" input options i.e. proportional to the number of comparisons performed before a match was found.mapapproach provided constant performance for all input options,switchapproach also provided constant performance for all input options, not expected but I'll assume some compiler optimisations there.switchapproach was slightly faster than themapapproach.switchandmapapproaches were both slower than theif else if...until you test the 3rd or 4th input options respectively.if else if...approach degraded in performance until the 11th input when it improved slightly and then continued to degrade again. (I can't explain this).In general, I prefer the
mapapproach over a largeif else if...block and 3 or 4 branches seems like a nice rule of thumb for when you might want to look at moving to themaporswitchapproach, both in terms of performance and readability, IMHO.Just to add some clarification, this Map.of method returns an immutable map of defined size, so there’s not a great deal of heap allocation.
Besides that I usually try to optimize code for readability, even if it’s at the expense of a few microseconds.
I also think that being a static content Map the compiler would probably do some kind of optimizations, but that’s only my guess and trust in the bright JDK engineers.
But I’d like to see that kind of performance comparison, always good to put theory to practice!
Fair points. I guess it also depends what you're trying to do.
If you're writing kernel code, or stuff for embedded systems, the issue of lots of branches can itself be an issue - from a performance point of view. Data oriented approaches might focus on removing branches in the first place by organising the layout of data and making the code run better by processing on 'chunks of stuff'.
A map or associative array of some kind probably makes heap allocations and has plenty of ifs in its internal implementation. So I would hazard a guess it is probably a lot slower than just using ugly ifs.
I second that.
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.
I don't like the Map version. My reason is basically that you're using a dynamic construct for a static purpose.
That translates to how easy it is to see what the code does. In the if-version, it's it's all right there and I know that when I read the code. When I read the Map-version, I need to find out where it's initialized and if I want to make sure that's all it does, I also need to make sure that no other code modifies the contents of the map.
Now, these problems can be minimized through developer discipline. However, especially when multiple people work on the same code or when the code and possibly the requirements change organically, discipline might lack in the end. Besides, if I'm new to your code, I don't know about the level of developer discipline, so I can't rely on it blindly.
Hi JasperHorn! Thanks for your feedback.
I think your point on using the dynamic construct for a static purpose is a valid one. But I don't agree with the discipline part, in fact I think it's exactly the other way around.
First of all, this Map.of method returns an immutable map, so there's no way it could be modified after initialisation. No worries about that, then.
Second, what I've seen the most in my life is indisciplined uses of if statements, and I think that's the source of most of the bugs I have found, and the most difficulties to read and understand code.
I would have no problem with a group of disciplined developers using lots of if statements, as I know they would abide by good code constraints. For a team of less experienced developers, I prefer to have them work under constraints that will help them organize their code and ideas.
Thanks for sharing!
Hi Thomaz,
I did notice the
Map.ofand realized that it returned an immutable map, but since your post never mentions any programming language, I responded to the language-agnostic advice rather than the Java example. Not all languages can express such immutability, and it wasn't actually mentioned outside of the example either.The other thing I meant to imply is that you have to keep the map initialization code close to the using the map. I think it's easy to forget the two are linked when you're writing unrelated code and you may end up placing that code between the initializing code and the using code, even when you're an experienced developer. It's not the kind of thing of thing that stands out in a code review either, I'd say. In my opinion, writing good ifs is about skill rather than discipline. Therefore, it's something people actually grow in. It's also something that stands out much more in a code review in my opinion.
(The difference I see between skill and discipline is that skill is a level you're at, closely related to experience, while discipline is a resource to be managed. A developer may or may not have different total discipline levels throughout their career, and they should probably learn to manage it better as they gain more experience, but that's not nearly as strongly coupled to experience, and there are always going to be moments of low discipline. On top of that, eliminating (or minimizing) the need for discipline wherever possible will free up more for where you do need it. I'm getting pretty philosophical here, though...)
When looking at the examples just now, there was something else that stood out to me. Note that this an observation without an opinion (it hasn't gestated enough to become an opinion yet) even if it may sound like an opinion. Both examples use enums. I used to like enums a lot a long time ago, but these days I don't use them much because I believe that in most cases they aren't the best tool for the job. Looking at the examples in that light, the second example looks to me like it would be better as three methods without arguments in which case there would neither be ifs nor a map. (The first example would still use the ifs or the map without the enum, I think.)
Thanks for the discussion so far! It's good to be able to debate things like this.
Personally, I found your Map code much easier to read than the ifs. I have always struggled with getting lost in repetition. The Map approach places proper emphasis on the relevant information, removing distraction. Thanks for this perspective.
Hi Marcus! 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!
You're welcome Marcus, thanks for your feedback!
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!
I really love these types of articles and the discussions that come from them. I absolutely love seeing other people's perspectives. It's facinating to see the contexts people attach to these discussions to further their point. I feel like I learn a lot from stuff like this. Thanks everyone who's participating in the discussion and of course Tomaz for kicking it off.
Thanks Chase, I really appreciate your kind words and feedback!
I'd have to say that I agree with this. If you have a program that requires many if-else statements and each produces a related result and those statements need to be ran again and again in a similar fashion but with different inputs, then using a map to mock these statements would make debugging easier.
Some comments may only be visible to logged-in visitors. Sign in to view all comments.