Law of Code Quality: Consistency

Imagine you have some MDBs (Message Driven Beans). You want to get rid of them because they are still EJB2 and they suck. You want to use Spring's JMS capabilities instead. Sounds quite good. But after some time, due to some budget problem or pending dead-line, you stop converting the old stuff "because it does not add value" (and which is almost guaranteed to happen). So you end up having MDB plus Spring JMS mixed throughout the code and maintenance people have to know both of them. It's difficult enough to know one of them, but now you end up having both solutions messed up on the long run.

Copy & Paste
In brownfield development you always have some code before you start. The look, quality and design of this code is very important for further developments. Extending complex legacy stuff often involves a copy-and-paste style of coding. (If this is preferable is a story of its own right but will not be discussed here.) In the context of maintenance I consider copy and paste a good habit because this way the existing conventions and patterns are obeyed, even if they are not documented. Of course the positive effects of copy paste are only achieved if the "right" piece of the system is copied. Like reference solutions in generator development, aka templates (Link MDSD Generator), the copied piece has to be of highest quality, i.e. according to conventions and guidelines defined for the application.

Implicit Conventions and Uniformity
If the guidelines and conventions of some software are not written down properly, the only documentation is the code itself. Even if there is decent documentation, capturing all aspects of software development is at best very difficult or can't be done at all. There are always some implicit conventions that are only available in the code. The more an application is uniformly satisfying these conventions and designs, the more pieces of it serve as "safe" templates for further extensions and modifications. This helps new members on the project to find their way around. And good (maintenance) developers see these implicit conventions in surrounding style and patterns, adapt to them and work according to them. Uniform code makes it easier for them to adapt to the new code base and "get to speed".

Broken Windows
As the pragmatic programmers toughed us in rule 4, broken windows are causing problems in real as well as virtual life. If the conventions are visible and clear, who would dare to stand out in breaking them? On the other hand, if e.g. a piece of code is formatted in three different ways, there is no shame in introducing a new style. People tend to stick with the things they know, because it is faster and feels more secure. (And usually we think it to be superior to things we do not know.) This leads to patchworks in the code. This mixture gets maintained (read copied) from time and time and all the broken windows get spread throughout the code base, the differences live on and grow.

So I postulate the 1st Law of Code Quality - Code Consistency. This applies to source layout, naming and other coding conventions, typical code fragments (also called idioms, most of the time some boilerplate code), design concerns, layering, architecture, used libraries, technologies etc. Consistency in the code is the most important issue. Failing to have a consistent code base will cause all the troubles known from mixed designs, mixed technologies, making it difficult to maintain and get new people into the team.

Living with Changes
Of course we have to change things again and again. It's easy to have consistency in simple conventions, like formatting, just use Eclipse format on the whole source tree and put Checkstyle into your build. Short code idioms like getting a database connection are more difficult. These are rarely documented, but once you have them unified, a Ruby or Groovy script can do almost any syntactical change to your code using powerful regular expressions. More complex changes, e.g. replacing EJB 2 with something else, is more involved. However just don't make the mistake to leave the old stuff as it is. No excuses about small budget, needed retest, pending dead-lines and such! If you can't convert the old stuff, leave it as it is. If you are too "weak" to change it, you earned staying with it. Bringing in new technologies needs a strong plan, better a script, to convert everything existing to the new style. All remaining code has to be changed to use the new technologies as they are supposed to. Typical idioms and best practices of that technology should be obvious at the end. You don't want a Java program to be coded like old C, a few classes with lots of static, monolithic methods, static data etc. The same is true for any refactoring.

A Brighter Future?
In keeping your code base consistent you need help. Especially in the beginning we need someone who flags new inconsistencies, reminds us of conventions. Use static code analysis as soon as you have identified a consistency target. (That might be as simple as grep *.java.) The proper (consistent) way to do something should be enforced from day one. Unfortunately documenting it is not enough. For layout use tools like Checkstyle. Other conventions and boilerplate code can be checked with tools, that support custom rule definition, e.g. Findbugs or PMD. Nowadays many tools come with a large number of base rules that cover common stuff. Most likely some are of use. Modularity and layering is enforced with reference checking, e.g. Macker or SonarJ. With some fantasy (and enough computing power at your build machine) you can create quite sophisticated checks.

equals and hashCode Generation

Recently we discussed equals() and hashCode() implementations. A proper implementation is not trivial, as "father" Bloch showed us years ago. Back in 2004 I wrote a plugin for Eclipse 2 to generate these methods. (Unfortunately I never managed to publish it. I know I am weak ;-) My home grown solution would produce something like

public int hashCode() {
  long bits;
  int result = 17;
  result = 37 * result + (aBoolean ? 1231 : 1237);
  result = 37 * result + (int)
    ((bits = Double.doubleToLongBits(aDouble)) ^ (bits >> 32));
  result = 37 * result + (int) (aLong ^ (aLong >> 32));
  result = 37 * result + anInt;
  if (anObject != null) {
    result = 37 * result + anObject.hashCode();
  }
  if (anArray != null) {
    result = 37 * result + anArray.hashCode();
  }
  return result;
}
public boolean equals(Object obj) {
  if (this == obj) {
    return true;
  }
  else if (obj == null || getClass() != obj.getClass()) {
    return false;
  }
  final Homegrown o = (Homegrown) obj;
  return (aBoolean == o.aBoolean &&
         aDouble == o.aDouble &&
         aLong == o.aLong &&
         anInt == o.anInt &&
         (anObject == o.anObject ||
           (anObject != null && anObject.equals(o.anObject))) &&
         (anArray == o.anArray ||
           (anArray != null && anArray.equals(o.anArray))));
}

I know, I know the implementation for arrays is most likely not what you want. (Did I say that I am weak? :-) Since Java 5 one could use the java.util.Arrays class to fix it. Nevertheless, it served me well for some years. There are several other plugins for Eclipse, and Scott McMaster wrote about in 2006. Since version 3.3 (Europa) Eclipse can finally do it on its own:

public int hashCode() {
  final int prime = 31;
  int result = 1;
  result = prime * result + (aBoolean ? 1231 : 1237);
  long temp;
  temp = Double.doubleToLongBits(aDouble);
  result = prime * result + (int) (temp ^ (temp >>> 32));
  result = prime * result + (int) (aLong ^ (aLong >>> 32));
  result = prime * result + anInt;
  result = prime * result +
                   ((anObject == null) ? 0 : anObject.hashCode());
  result = prime * result + Arrays.hashCode(anArray);
  return result;
}
public boolean equals(Object obj) {
  if (this == obj) {
    return true;
  }
  if (obj == null) {
    return false;
  }
  if (getClass() != obj.getClass()) {
    return false;
  }
  final Eclipse33Java5 other = (Eclipse33Java5) obj;
  if (aBoolean != other.aBoolean) {
    return false;
  }
  if (Double.doubleToLongBits(aDouble) !=
      Double.doubleToLongBits(other.aDouble)) {
    return false;
  }
  if (aLong != other.aLong) {
    return false;
  }
  if (anInt != other.anInt) {
    return false;
  }
  if (anObject == null) {
    if (other.anObject != null) {
      return false;
    }
  }
  else if (!anObject.equals(other.anObject)) {
    return false;
  }
  if (!Arrays.equals(anArray, other.anArray)) {
    return false;
  }
  return true;
}

Using the ternary operator the hashCode method gets quite compact, but equals is a bit too verbose for my liking. IntelliJ IDEA could always generate these methods. IDEA 7.0 creates something like

public int hashCode() {
  int result;
  long temp;
  result = (aBoolean ? 1 : 0);
  temp = aDouble != +0.0d ? Double.doubleToLongBits(aDouble) : 0L;
  result = 31 * result + (int) (temp ^ (temp >>> 32));
  result = 31 * result + (int) (aLong ^ (aLong >>> 32));
  result = 31 * result + anInt;
  result = 31 * result +
                (anObject != null ? anObject.hashCode() : 0);
  result = 31 * result +
                (anArray != null ? Arrays.hashCode(anArray) : 0);
  return result;
}
public boolean equals(Object o) {
  if (this == o) {
    return true;
  }
  if (o == null || getClass() != o.getClass()) {
    return false;
  }
  Idea70Java5 original = (Idea70Java5) o;
  if (aBoolean != original.aBoolean) {
    return false;
  }
  if (Double.compare(original.aDouble, aDouble) != 0) {
    return false;
  }
  if (aLong != original.aLong) {
    return false;
  }
  if (anInt != original.anInt) {
    return false;
  }
  if (anObject != null ? !anObject.equals(original.anObject) :
                         original.anObject != null) {
    return false;
  }
  // Probably incorrect - comparing Object[] with Arrays.equals
  if (!Arrays.equals(anArray, original.anArray)) {
    return false;
  }
  return true;
}

Typical IDEA, with a little fix for +0.0/-0.0 and some warning concerning Arrays.equals, but else totally the same. In fact, all these implementations suck (including my own, which sucks most). All these result = prime * result ... and if ... return false; are definitely not DRY. I always favour Apache Commons Lang builders. A hand-coded solution using them would look like

public int hashCode() {
  return new HashCodeBuilder().
         append(aBoolean).
         append(aDouble).
         append(aLong).
         append(anInt).
         append(anObject).toHashCode();
}
public boolean equals(Object other) {
  if (this == other) {
    return true;
  }
  if (other == null || getClass() != other.getClass()) {
    return false;
  }
  ApacheCommons o = (ApacheCommons) other;
  return new EqualsBuilder().
         append(aBoolean, o.aBoolean).
         append(aDouble, o.aDouble).
         append(aLong, o.aLong).
         append(anInt, o.anInt).
         append(anObject, o.anObject).
         append(anArray, o.anArray).isEquals();
}

Well, that's much shorter, isn't it.

Generic Build Server Notification Tray

I like to be notified immediately when our build fails (or at least before someone notices and tells me that it's my fault :-). I really have to know. In fact I am kind of paranoid about it. Ideal would be some kind of system tray notifier, like Team City has one.

Some years ago at Herold we were using Anthill OS, which is nice but minimalistic and did not offer any notifications. So I made a tray notifier myself using the System Tray Functionality of Java SE 6. The notifier polled the configured build server status page and used regular expressions from a property file to parse it. If a build changed to red then a little popup was shown. Later I was using Cruise Control and the only thing I was able to find was the CruiseControl-Eclipse-Plugin on Google Code. That's quite cool stuff, but I needed something that popped up in my face when the build was red.

So here is my Generic Build Server System Tray Notifier. After unpacking the zip you have to create a startup script or link executing the jar that looks like <path to java 6>\bin\javaw.exe -jar BuildServerSystemTray.jar <path to config> (Source is included in the zip.)

Configuration
The notifier is generic and needs a properties file. The zip contains sample configurations for Anthill OS 1.7, Cruise Control 2.3 and Hudson 1.2. You will have to customise the configuration. At least the build server URL (server.url) has to be set accordingly. It should be easy to create configurations for other build servers, just set the proper value for the status.pattern property. This property defines a regular expression matching the whole information about a build containing the project name, success or failure and build time. The regex grouping values status.name.group, status.value.group and status.date.group must be set accordingly.

FAQ
Q: I was checking out the source, why is the package at.kugel used as top level namespace instead of org.codecop? A: Kugel was my coder pseudonym in the 80ties when I started coding for Commodore 64. I'm using it from time to time when I'm feeling retro. Kugel is German for ball or sphere. The name was coined by my first coding buddy because I was quite overweight.