Original URL: http://www.theregister.co.uk/2008/05/05/emergent_design_part_four/

Read, test, don't repeat - how to avoid code complexity

Emergent Design: Lessons from Y2K

By Scott Bain

Posted in Software, 5th May 2008 12:02 GMT

Book extract, part four Redundancy, testability and readability are key to building simple and maintainable code. In the fourth extract from his book, Emergent Design: The Evolutionary Nature of Professional Software Development, published by Addison Wesley, Scott Bain tackles the problems and principles involved.

We all saw an example of redundancy rearing its nasty head just a few years ago.

The Y2K remediation effort cost countless billions of dollars, scared the heck out of entire countries, and in general disrupted progress in the technology sector. Many companies basically stopped doing any kind of new projects in the latter part of 1999, because they were putting their resources into fixing the Y2K bug.

What made Y2K hard to fix? Was it a complex problem? No! Not at all. Systems all over the world had stored the year portion of the date in innumerable records as a two-digit number, rather than a four-digit number. Not a complex problem at all.

Except that it appeared in billions of places, all of which had to be found and changed.

We can criticize the programmers who wrote the original code this way, but the fact is I find redundancies in my own code and the code of colleagues all the time. It is easy to do; it tends to creep in on you, especially when you are creating an extension to a new system. Also, some aspects of the languages we use can promote redundancy. Consider the following code:

public interface Weapon{
  public void load(int rounds);
  public int fire();
  public void setSafety(boolean on);  
}

public class Pistol implements Weapon{
  private int myBullets;
  private boolean safety = true;

  public void load(int bullets){
    if(bullets<=6){
       myBullets = bullets;
    } else {
      System.out.println("Pistol only holds 6 bullets");
    }
  }

  public int fire(){
    int rval = 0;
    if(safety){
       System.out.println("The Safety is on");
    } else {
       if(myBullets > 0) {
          System.out.println("Bang!");
          myBullets = myBullets - 1;
          rval = 10;
       } else System.out.println("Click!");
    }
    return rval;
  }

  public void setSafety(boolean aSetting){
    safety = aSetting;
  }
}

public class TommyGun implements Weapon{
  private int myBullets;
  private boolean safety = true;

  public void load(int bullets){
    if(bullets<=50){
       myBullets = bullets;
    } else {
      System.out.println("TommyGun only holds 50 bullets");
    }
  }

  public int fire(){
    int rval = 0;
    if(safety){
       System.out.println("The Safety is on");
    } else {
       if(myBullets > 9) {
          System.out.println("Budda Budda Budda!");
          myBullets = myBullets - 10;
          rval = 100;
       } else System.out.println("Click!");
    }
    return rval;
  }

  public void setSafety(boolean aSetting){
    safety = aSetting;
  }
}

There are lots of redundancies here - the way setSafety() is completely identical in both Pistol and TommyGun, for example. If I want to change the way this works - make the state persistent by writing it to the disk every time, for instance - then I must remember to change it in both places.

The Java and .Net interface type leads me to this inherently; it tends to create redundancies among the implementing classes. This is because the interface cannot have any actual implementation, so I cannot put common implementation into it, and allow the implementing classes to inherit.

What if I used an abstract class instead? Look at the following code:

public abstract class Weapon{
  protected int myBullets;
  protected boolean safety = true;  

  public abstract void load(int rounds);
  public abstract int fire();

  public void setSafety(boolean aSetting){
    safety = aSetting;
  }

}

public class Pistol extends Weapon{

  public void load(int bullets){
    if(bullets<=6){
       myBullets = bullets;
    } else {
      System.out.println("Pistol only holds 6 bullets");
    }
  }

  public int fire(){
    int rval = 0;
    if(safety){
       System.out.println("The Safety is on");
    } else {
       if(myBullets > 0) {
          System.out.println("Bang!");
          myBullets = myBullets - 1;
          rval = 10;
       } else System.out.println("Click!");
    }
    return rval;
  }
}

public class TommyGun extends Weapon{
  
  public void load(int bullets){
    if(bullets<=50){
       myBullets = bullets;
    } else {
      System.out.println("TommyGun only holds 50 bullets");
    }
  }

  public int fire(){
    int rval = 0;
    if(safety){
       System.out.println("The Safety is on");
    } else {
       if(myBullets > 9) {
          System.out.println("Budda Budda Budda!");
          myBullets = myBullets - 10;
          rval = 100;
       } else System.out.println("Click!");
    }
    return rval;
  }
}

I have eliminated some of the redundancy already: The setSaftey() method is now in one place, inherited by both Pistol and TommyGun. Also, the data members myBullets and safety were common, so I put them in the superclass too and made them protected so the subclasses could still access them directly.

There's more I could do, of course. The weapons both operate in a conceptually similar way, only the details vary. If I am lucky enough to know the Template Method pattern, I could pretty easily get rid of all the other bits of redundancy here, without sacrificing any of the uniqueness of these two weapons.

Redundancy and coupling

But haven't I introduced coupling in order to deal with the redundancy problem? The abstract superclass puts one rule in one place, but it also means that a change in the superclass (Weapon) will have an effect on the subclasses (Pistol and TommyGun). This is inheritance coupling, certainly, so have I traded one problem for another?

page break

No, not really. Remember accidental coupling is coupling that arises unintentionally, or for a misguided, unnecessary reason. Intentional coupling is coupling I intend, and that helps my project. I am generally not stupid, and neither are you. What we do on purpose is usually better than what happens beneath our notice.

Here, I am introducing inheritance coupling in order to solve the redundancy problem. I want changes in Weapon to propagate down to Pistol and TommyGun. It is my intent that this happen: it is no accident, and I am not liable to forget that it is going to happen. I am going to depend on it, in fact.

Protected data members

In my gun-toting example, I used protected members myBullets and safety in the abstract class Weapon, so that I could access them via inheritance in the same way that I had been accessing them when they were local, private members of Pistol and TommyGun.

My next step is to change this. I would make myBullets and safety private, and then declare protected or public methods to get() them and set() them. Why?

We said that inheritance creates coupling. That can be a good thing, as it is in the case of eliminating the redundancies we have dealt with. But a bad kind of inheritance coupling can emerge with protected data members.

With protected data members, subclasses are coupled to the existence of those members. If I later want to store the bullets outside the Weapon object, maybe in a Clip or Magazine object, I will now have to change Pistol, TommyGun, and whatever other subclasses I have created to now call methods to get the bullet amount rather than accessing myBullets directly. If I make a getBullets() method in Weapon, then make Pistol, TommyGun, and so on call it to get the bullet amount, then I can change the way it is stored and retrieved in that one place, getBullets().

Also, with get() and set() methods, I can create read-only data members (just do not provide a set() method), or write-only, or I can put validating code into the set(), and so on. This is such an important thing that it has lead to the creation of an entirely new language feature in .Net: the property.

Generally, protected data members are a bad idea. Public data members are even worse. With that said, design at this level is often a balancing act between coupling and redundancy. The key is to have a considered reason behind your decision to keep things in the subclasses (at the cost of redundancy) or to put them in the superclass (at the cost of coupling them).

If you have a sensible motivation that drives this decision, that sensibility will likely mean that the approach is clear and explicit, and will not cause maintenance problems. Furthermore, the decisions you make are not set in stone. Remember, you are embarking on an evolutionary process here. You can expect to change things as you learn more about the project, as the requirements expand and change, and as you have new, better ideas.

Refactoring, and adherence to your coding principles, will give you the confidence to make changes when this occurs, and your understanding of design will allow you to see the opportunities that arise for improved design that may result from the change process.

In other words, an emergent design.

Testability

One thing I have learned in recent years is that unit testing actually has great value in terms of evaluating code against these principles. As a consultant, I am usually called in when things are not going well. Companies rarely call for extra help when everything is fine, after all. Consultants add a lot of expense.

In order to come up to speed on the team's activities, goals, and current situation, I need something to work with. Asking for design and requirements documentation is usually a forlorn hope. If the team is really in trouble, how up-to-date do you suppose these documents are?

However, I have noticed that reading unit tests can be very revealing. In a way, they are a record of the expectation of the developer or tester who wrote them, and therefore can reveal a lot about what a class is supposed to do.

Usually, one of the first questions I ask a customer's development team is whether it is unit testing. Generally, the answer is "no".

Furthermore, when I suggest that unit testing might be a good idea, I generally encounter a lot of resistance, with the excuses that unit testing is "too hard", "too time-consuming", "frustrating", and that it does not bring enough value for the trouble.

At first, I would argue with them, until I tried to add tests to their projects, finding that it was all those things and more. This puzzled me, because tests that I wrote simultaneously with the coding were so were much easier, and were clearly worth the time and effort it took to create them. It turns out that code is hard to test when it is not designed to be testable in the first place, and so adding unit tests to an existing code base is awfully difficult.

page break

But why? What is it about bad code that makes it hard to test?

Examining this question over and over again with different customer teams has been incredibly revealing. Some of the most common reasons are:

All of these represent design problems anyway! In the first case, I would suggest that there is too much coupling in the system if units cannot be tested in isolation (or with only a few other units in play). In the second case, I would argue that a class with multiple responsibilities is weakly cohesive in the first place, and will therefore be overly complex and poorly encapsulated. In the third case, if an issue is repeated in the system, there is a redundancy, and I know I do not want that.

In other words, considering the testability of the code I have written (or better yet, am about to write) is a great way to evaluate its quality, even if I cannot quite see how it stacks up in terms of coupling, cohesion, redundancy, and so on. If I cannot see a way to test it, maybe I should rethink what I am planning on doing.

It is like a first principle of first principles. It is one thing I can consider, which will cause me to consider other good things automatically.

Readability

Cohesion, coupling, and redundancy are like the three legs of a stool: you need to address them all if you want reliability and stability. For a while, in my courses and when consulting or mentoring, these three were the focus of my attention when it came to coding principles.

However, I have added a fourth concern recently, after having worked with teams that did not address it well, and seeing the results. Code needs to be readable.

Software developers tend to be intellectually minded people who pride themselves on the complexity of their thoughts, on being able to understand subtle, intricate, even arcane writings and discussions. It is a stimulating mental challenge to them to see how much they can do with a single line of code, or how cleverly they can accomplish a mundane task. Readability is for wimps, right? I have actually heard developers say things like: "If you can't read my code, you should not be messing with it," or "I can read my code, and that is all that matters."

The economics of software development say otherwise.

First of all, you may not be the only person who has to read your code. It is quite likely that another developer will have to maintain it in the future after you have been promoted, moved on to another project, or retired to your island in the Caribbean.

Secondly, you may have to read the code in a few months, or even in a few years, when other projects and other clever code have taken over your active memory, and at that point it can seem like reading something written by someone other than yourself.

Also, often good software is a reflection of good teamwork; your ideas may not always be the only ones, or the best ones for the part of the project you are working on. If other people cannot understand your code, how can they contribute their ideas to it? The synergy that comes from allowing multiple minds to address the same problem can be very powerful, and can lead to efficiencies that a single developer alone cannot achieve.

Next time, in our final installment, we look at the pathologies in code that indicated when redundancy, testability and readability have not been attended to.

This chapter is excerpted from the new book, Emergent Design: The Evolutionary Nature of Professional Software Development by Scott Bain, published by Addison-Wesley Professional, March 2008 ISBN 0-321-50936-6 Copyright (c) 2008 Pearson Education, Inc. For more information, please see informIT.com and Register Books.