Introduction

If you have read my previous articles, or know me personally, you must know that the one kind of bad code I hate the most is if-else trees. In the first article of this weblog, I talked about if-else trees in general, exposing some general ideas about how to avoid them, remove them from the code or why they are bad, describing them as a code smell.

Today, as I have just finished writing a series of articles in French about the SOLID principles, I am going to focus on the maintainability aspect of if-else trees, and make them fight against two of the SOLID principles: the Single Responsibility Principle and the Open/Closed Principle. This will allow me to introduce some important rules to know about if-else trees.

 Definitions

A typical if-else tree has several responsibilities. When you are looking at an if-else tree, you are very often seeing a god object. Indeed, as the if-else tree knows many things on “when to do”, with all its conditions, it might very likely know many things on “how to do” as well.

Rule #2 : When you are looking at an if-else tree, you are very often seeing a god object

The example if-else tree is definitely a one-case-one-branch tree (OCOB tree), the kind of if-else tree where every case has its own conditional block. When dealing with an OCOB tree, one can be pretty sure that the Single Responsibility Principle is not applied. Indeed, OCOB trees are by definition an aggregate of responsibilities packed together using an if-else tree.

With our example, the description comment of the class is nearly sufficient : “This class handles all forms of incoming HTTP requests by constructing a proper HTTP response.” In fact this is the if-else tree that “handles all forms of incoming HTTP requests”, and each if is for one of them.

Robert C. Martin introduces the Single Responsibility Principle with this statement:

There should never be more than one reason for a class to change.

Here there is a responsibility – a reason for changing the class – associated with each block of the tree. From the beginning of the example tree, we have:

• outputting a page to the HTML console
• retrieving a file in general
• retrieving a thumbnail
• retrieving a subtitle
• retrieving a regular file
• notifying plugins
• generating a response
• outputting a picture
• browsing files
• …

…and the Single Responsibility Principle is obviously not applied here. This is not the point of this article but in this particular situation, one could have used the Chain of Responsibility pattern, thus distributing the responsibilities among the different handlers, each of them responsible for only one kind of request. This is what is done to handle HTTP requests in about every existing framework in any language. For Java, readers might want to have a look at org.apache.http as an example implementation.

Rule #3 : A one-case-one-branch tree can often be split using a chain of responsibility

Open/Closed Principle vs if-else trees

If-else trees often break the Open/Closed Principle, creating excessive coupling and maintainability issues. If-else trees break the Open/Closed Principle when some conditions are based on the knowledge of the implementation details instead of based on abstraction. This can be a direct and heavy type checking, or something more subtle based on the knowledge of a particular post-condition, etc. The latter is often found with a comment like “Being in this situation means that the actual handler was XYZ” or “As xyz was empty before the call, we can tell for sure that Foo did blabla…“, which expresses the developer’s guilt-feelings for writing such a condition; and is definitely a code smell.

Rule #4 : In an if-else tree, an over-commented block of code might be the expression of guilt-feelings from the developer, indicating a potential code/design problem.

In the example we can find a couple of such comments, for example:

//hacky stuff. replace the png icon by a jpeg one. Like mpeg2 remux,
//really need a proper format compatibility list by renderer

or

// with the new parser, files are parsed and analyzed *before* creating the DLNA tree,
// every 10 items (the ps3 asks 10 by 10),
// so we do not know exactly the total number of items in the DLNA folder to send
// (regular files, plus the #transcode folder, maybe the #imdb one, also files can be
// invalidated and hidden if format is broken or encrypted, etc.).
// let’s send a fake total size to force the renderer to ask following items

Definitely a design issue here as the RequestV2 class has to know hyper-detailed details on the implementation of other classes to work properly.

Do not build if-else trees, ever!

Before building an if-else tree, you should always ask yourself whether there is a design issue somewhere. You should never feel like you have to build such a tree as this nearly always implies that the class will know too much of something. This can be too many responsibilities or too many implementation details, as we’ve just seen in this article, but this can also be a too big business : an apparently unique responsibility hiding lots of sub-responsibilities that can be split.

Rules #5 : An if-else tree always knows too much

Conclusion

Thank you for reading this article, which is not the last one on the subject of code maintainability/scalability/evolutionability. Indeed, although this need of code maintainability has always been around, the cloud computing/agile development era brings it to the forefront. In a world where more and more,we need to develop new features and make existing ones evolve at the speed of light, maintainability will become the number-one rule of every company, and if-else trees do not fit. They simply do not belong to this new era.

Further readings

I’m feeling lazy today, so a pure wikipedia “further readings” section: