Code Smells Taxonomy

A short taxonomy of code smells here.

TDD Anti-patterns

James Carr has a post on tdd anti-patterns in his blog. I'm sure you will recognise a few.

Software Quality from Systems Thinking Perspective

Lately I was in several conversations about software quality, trust and office politics. It seems that the standard argument is that we should strive to write quality software, but in the real world we have to make compromises. Moreover, we (as developers) cannot assume the responsibility for technical decisions when there are higher interests at stake.

I don't want to repeat what Ryan said before. Instead, I wanted to look at the problem from a systems thinking perspective.

Code base with a low amount of technical debt is a healthy code base. It seldom happens that it's worth acquiring technical debt. It's very hard to estimate the amount of effort required to remove it, making it an incalculable risk, an uncertainty in the code base. For a longer discussion see Martin Fowler's post on technical debt.

A healthy code base allows developers working with it to provide reliable estimates about the amount of effort required to implement a feature. Also, a lesser number of unknowns in the code base gives developers ability to work at a predictable pace while maintaining quality. Better estimates and a stable rate of progress means developers are more likely to complete the stories they committed themselves to for a given iteration.

Keeping promises is a good way to gain trust; completing tasks that developers promised to deliver builds up trust with the product owner/customer team (and possibly within other parts of the company). When a product owner has a high degree of trust in the development team, she is more comfortable with leaving technical decisions in the hands of developers. This completes the circle: since developers are entrusted with the making of technical decisions, they can maintain healthy code that is high in quality.

In systems thinking this pattern is known as "Reinforcing Loop". The above example shows the loop that improves code quality and builds up trust in developers over time. Such a loop can also amplify negative changes.



In the presence of technical debt in the code base it's hard (if not impossible) for a developer to provide a reliable estimate. For the same reason a predictable rate of progress is hard to achieve - the amount of actual work is unknown. Depending on the size of the technical debt developers are likely to find it hard to deliver on their commitments. In such a situation developers will be under pressure from other developers on the team (and oftentimes the developer herself) and from the product owner (and possibly other parts of the organization) to make shortcuts (or in other words to increase the technical debt). Lack of trust in developers, and uncertainty in the amount of work required to improve quality and health of code base, will make a product owner hesitant to allocate time in the iteration for such work.

There is a concept of leverage in systems thinking - a sometimes small, well focused action that can produce significant, enduring improvements. Code health provides leverage that developers can use to bring improvements into software development process. By maintaining code health at a high level we (developers) create work conditions where developers are trusted and can continue to improve code health. By sacrificing quality we get trapped into a vicious cycle of eroding code health and vanishing trust.

Double-checked locking doesn't work in Java

This doesn't work in Java:

class Foo {
 private Helper helper = null;

 public Helper getHelper() {
  if (helper == null)
    synchronized(this) {
      if (helper == null)
        helper = new Helper();
    }
  return helper;
 }
}

It is possible that to due to compiler-based reorderings helper reference will be initialised while the fields in Helper object will be at their default values. There are other more subtle problems with the double-checked locking approach. You can read the complete paper here.

In most cases such an optimisation is not worth the trouble. If you want to lazy-initialise a singleton, define it as a static field in another class:

class Helper {
 class HelperSingleton {
   static Helper singleton = new Helper();
 }

 public Helper getHelper() {
   return HelperSingleton.singleton;
 }
}

The semantics of Java guarantees that singleton field is going to be initialised only when referenced, and that all threads accessing the field will see the writes performed during the initialisation.

Keeping tests useful

There is a subtle difference between writing production code and test code. Both have to be intention-revealing and maintainable, but good tests are also specifications. As such, tests have to be crystal clear about what's being tested and show the usage of api/code being tested.

Quite a few times, I've seen test-data, setup for mocks, and assertions being tucked (far) away from the actual test, all in the name of maintainability. Hiding these things away makes reading tests harder, and can hide problems with the production code. Imagine a situation when you have (oh, no!) a constructor with ten parameters. To make tests more maintainable, you create a 'helper' method that returns a new instance of the class, and requires only three parameters. You just saved yourself from tons of copy-paste code-reuse, right?

That's certainly one way to look at the problem. However, why does the constructor need ten parameters? Could it be the case that the class has too many responsibilities? Now it's hard to tell; the tests are hiding the problem. This makes them less *useful* than they could be.

Next time you're writing tests and find yourself creating shortcuts for repetitive bits of code or heavy setup, pause and say to yourself: "Hmmm, how interesting...". Perhaps a better way to solve the problem will present itself.

Thanks to Ryan Cooper "Agile Guru" for help with this post.