April 17, 2014

Clean code - why it matters

Well designed and thought through code is something every software professional should take seriously as it is an enabler for so many important properties of software design. It will help you in areas such as testability, maintainability, modularity, sustainability, readability, security, time-to-market and so on. The list of positive side effects of creating well designed code is neverending.

If you are new to the term clean code, then Bob Martin's book Clean Code is a good place to start to get an inroduction to what it means. In his book, Bob Martin shows many examples of what clean code can look like and you may not agree with all of them but the importance is to understand the overall concept and purpose of the term. Other areas to explore in terms of code design is domain driven design (DDD) and test driven development (TDD).

In my view, well designed code is the foundation of everything that we do as software professionals. All contemporary best practices that we apply daily are dependent of well designed and expressive code. Whether it be TDD, DDD, domain driven security, continuous integration, scalability and so on. A poorly designed code base that is nothing more than a big mess of code will never enable you to succeed in any best-practice, at all. Period. This may sound a bit harsh but in the long run I really do believe that well designed code is the base for everything else.

For example, code with traits such as encapsulation and inversion of control will be more testable. And in this day and age I think most developers realizes the benefits of having well tested code.

Another example is that code that use language features together with wording/naming of methods and variables to express its intent and purpose will be more readable and understandable. This, in turn, will make it easier to maintain and improve the code. If the code is easy to read it will also be easier to spot mistakes or problems in the design.

Software security is an example of an area where you want to make it as easy as possible to spot mistakes and bugs. Security related code should ideally have the highest standards when it comes to being readable and explicit in its intent. Critical code should be the cleanest code. Too many times have I seen security related code that is nothing more than a huge mess and, in general, it tends to be in the messy code where you will find the most bugs. My colleague Dan Bergh Johnsson has some excellent thoughts on how code design can help improve software security (Domain Driven Security) and I suggest reading it if you are interested in software security.

A tale from the trenches

Making the code readable, and being explicit with its intent and purpose, can also help you avoid problems that you didn't even think about. To give a concrete example: I was reviewing some code for a client and it turned out that the code didn't compile when upgrading to a newer JDK version. At first it wasn't clear to me why it refused to compile but after sifting through some JDK documentation it turned out that the code stopped working because a bug in the Java compiler had been fixed in the newer version.1 I.e. the code should never have worked in the first place. The code looked something like this (turned into a simple example to protect the innocent):
public class A {
    private static class Inner {
        private int value() {
            return 42;
        }
    }

    public <T extends Inner> void doStuff(T t) {
        final int value = t.value();
        // ...
    }
}

This code will compile in JDK 5 and 6 but not in +7. Can you spot the mistake? As a Java developer this code looks legit as it is allowed in Java to access a private member of an inner class. The key here is <T extends Inner>. If the method doStuff() would have looked like below it would have been valid.
public void doStuff(Inner t) {
    final int value = t.value();
    // ...
}

The reason for why the first example is invalid is that T is a subclass of Inner, and as such there is no guarantee that a private member of Inner will be available in T. Which makes perfect sense when you think about it.

Now how does this relate to clean code? Well, my point here is that if we had been thinking about making the code expressive, and to clearly communicate its intent, we could have avoided getting into this situation in the first place. Let me elaborate.
Class Inner is an inner class that is only to be used by class A. That is a clear intent and it is expressed by declaring Inner as private. But even though Java allows A to access private members of Inner (by use of synthetic accessors) it is better to avoid that, and instead design Inner just as if it wasn't an inner class. That means that if we intend the method value() to be used by A then it should have a greater visibility. E.g. public. Any method that is made private should not be accessed outside of Inner. This is called encapsulation and we should apply and honor it just as much for inner classes as we do (and are forced to do by the compiler) for normal classes.

So if we take the code example above and make it more expressive then it could look like this. And we could have avoided the compatibility issues with the compiler.
public class A {
    private static class Inner {
        public int value() {
            return 42;
        }
    }

    public <T extends Inner> void doStuff(T t) {
        final int value = t.value();
        // ...
    }
}

Clean code matters

Clean code matters. It is the foundation of, and an enabler for, so many areas in software development that every professional should be consciously aware of it. And as we have seen, not only will it help you achieve what you are aiming for. It will also help you avoid getting into situations and issues that you didn't think of.

---------
Updated Apr 18, 2014: Added third code example.

1. See bug JDK-6246814

1 comment: