Redneck conditional anti-pattern

Sometimes I encounter weird looking if statements. By weird, I'm thinking of multiple negations all over the code base. Just a few days ago together with my colleague we found them very exotic during debug of an external library. From this point on, I'll call the following anti-pattern redneck negation or "I ain't not going to do that".

 So let's take a look at a snippet:
if(!some_flag_that_means_off) {
   //handle negative scenario where we change the flag value
} else {
   //handle positive scenario where we also change the flag value
//decide upon my flag
The simpler version would be an extracted method with:
 if(some_condition) {
    //return positive
 } else {
    //return negative
Now the code is easier to read and simpler to maintain.

State of mind

You might be wondering how someone writes this or how we end up with this type of multiple negation codes. It is easy to judge people, but the reality is that this might have been done at some crazy pressure to "save time". It is possible the programmer working on this part was solving some bug and did not consider the readability. It could be also the lack of knowledge or attention or maybe cosmic rays. No matter the reasoned judgment is not the answer.

I once had a discussion with a friend about a related topic and he said on his team they have banned the debugger. This might seem crazy at first, but when you think about it, we usually add various redundant if's during debugging. We add them without seeing the bigger picture and that is one other way we can end up with conditionals all over the place. Now his argument against the debugger looks less crazy.

While not all if's are made the same, the vast majority of them can be avoided and the complexity they bring with them. Of course, this is a broader topic but it is something to keep in mind.

How do we fix this?

Even though this should be simple enough to change I would first start with the following:
Whenever I do refactoring, the first step is always the same. I need to build a solid set of tests for that section of code. The tests are essential because even though I follow refactorings structured to avoid most of the opportunities for introducing bugs, I'm still human and still make mistakes. Thus I need solid tests.
Martin Fowler - Refactoring: Improving the Design of Existing Code
It is truly one of the best advice someone can give you before you start refactoring. The statement switch from negative to positive should be simple enough to start off with. Then again, it should be simple enough to not even introduce this but that is obviously not the case.
Next step would be to remove the changing of the flag either by extracting this into a separate method or even possibly removing it altogether.

There are plenty of options in the simplification of the conditionals. One way is to [replace conditional] with polymorphism. There is a great talk by Misko Hervey part of the Google Clean Code Talks titled "Inheritance, Polymorphism, & Testing". Also, we can run into the conditionals arrow anti-pattern and the related text from Jeff Atwood titled Flattening Arrow Code.

Popular Posts