DEV Community

Discussion on: Keeping your code clean by sweeping out "if" statements

Collapse
 
jpivarski profile image
Jim Pivarski

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.

Collapse
 
tomazfernandes profile image
Tomaz Lemos

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!

Collapse
 
jpivarski profile image
Jim Pivarski

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.

Thread Thread
 
tomazfernandes profile image
Tomaz Lemos • Edited

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!