Skip to main content

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 from this blog

HTML 5 data-* attributes, how to use them and why

It is always tempting to add custom attributes in HTML so that you can use the data stored there to do X. But if you do that there is no way of knowing if your HTML attribute will not be overridden in the future and used for something else and additionally you will not be writing valid HTML markup that can pass HTML 5 validator and with that you can create some very bad side effects. That is why there is a spec in HTML 5 called custom data attributes that enable number of useful features.

You may go around and read the specs, but the basic idea is very simple, you can add any attribute that starts with "data-" and that attribute will be treated as non-visible data for that attribute. By non-visible I mean that it is not something that gets rendered to the client so it does not affect the layout or style of the page, but it is there in the HTML so in no way this is private.
So let's get right into it, the following snippet is a valid HTML5 markup

<div id="aweso…

Basic Authentication with RestTemplate

Spring Rest Templates are very good way of writing REST clients. By default they work with basic HTTP so if we need to use Basic Authorization we would need to init the rest template with custom HttpClient. This way the Rest Template will automatically use Basic Auth and append to the HTTP headers "Authorization: Basic BASE64ENCODED_USER_PASS".

HttpClient client = new HttpClient(); UsernamePasswordCredentials credentials = new UsernamePasswordCredentials("USERNAME","PASS"); client.getState().setCredentials( new AuthScope("www.example.com", 9090, AuthScope.ANY_REALM), credentials); CommonsClientHttpRequestFactory commons = new CommonsClientHttpRequestFactory(client); RestTemplate template = new RestTemplate(commons); SomeObject result = template.getForObject( "http://www.example.com:9090/",SomeObject.class );

In EE application this would probably be managed by DI framework like Spring Core and only initialized once sin…

Temporary files and directories in Java 7 and before

Sometimes we want to create a temporary file, whether to save some data that gets written by some other application or just to temporary store stuff. Well, usually applications have their own temporary folder where they do this and it gets somehow configured. But why not use the underlying OS specific file like "/tmp/" in Linux so there must be some system property that has this info and there is. The key is "java.io.tmpdir" resulting in "/tmp" in my case or by code:
String tempDir = System.getProperty("java.io.tmpdir"); We can use tempDir  folder as a temporary place to store files, but there are a lot nicer ways to work with files like this even in JDK6 not just in JDK7:
import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; public class TempFile { public static void main(String[] args) { try { // create a temp file File tempFile = File.createTempFile("old-file",…