Comment Stories

 This article was posted originally on JRoller on April 27, 2005.

Comments are meant to be read. Sometime, they tell a story. The story of the code that follows them. Using Version Control tool, you can even retrieve the main characters.

Comment Story 1: John and Eduard

During a design meeting, John explains to Eduard how to modify the code he wrote, to include a new feature. They agree that some switch statement would be needed at some point in the class. But Eduard does not like switch statements. Eduard prefers the long list of if/else statements. He is a fervent Conditional Spaghetti adept. So he writes the code his own way, and include the following comment at the top of its code:

// sorry John, I hate the case structure...

Comment Story 2: Robi and Andras

Robi likes complex code. He likes long methods and overusing design patterns. His pride is a 400 lines long method, introduced by the following comment:

//the fair dinkum! select appropriate editor, handle selections, wash, iron, f*ck etc.

Robi leaves the project, and his code is handed over to Andras for maintenance. Andras has to fix bugs in Robi's code, and he doesn't like it. He has several sleepless nights, dreaming about monster methods managing everything in his house, ranging from dish washing to bringing down the garbage. For fear of breaking anything, Andras will try to change as less code as possible in Robi's piece of art. On one inspired day, he will add the following comment at the beginning of a 2500 lines of code long class:

/**
 * @author  K. Robi
 * ^^^^^^^^^^^^^^^^ you can tell ;-)
 */

Comment Story 3: Robi is angry
Robi works with JTables. He is trying to make something not really easy: the table should sort if you click on a column, and an arrow should show on the table header on which column and in which direction it is sorting. Being a big Design Pattern fan, he is putting quite some of them, and of course over-complicating the whole design. But this is not the point here. The point is that for some reason, something does not work when the reordering of columns is not allowed. Was it a Java bug? Or is it a misunderstood feature of JTable? I don't know, but what I know is that Robi spent some time on it. You can tell because he left several lines of code commented out:

//                    getTableHeader().invalidate();
//                    validateTree();
//                    invalidate();
//                    revalidate();
//                    repaint();

And then he got angry. REALLY angry. So angry that he felt he has to put the following comment in his code:

    //some fucking ugly workaround for the case when fucking column reordering is not allowed.
    //damn fucking strange, but none of these fucks above work and i'm fucking tired of fucking around
    //with these fucked-up tables! JTable fucks!
    //fuck that!
                

All this followed by those mysterious lines of code that I would be too scared to remove:

                    if(!getTableHeader().getReorderingAllowed())
                    {
                        TableColumn col1 = getColumnModel().getColumn(0);
                        int w = col1.getWidth();
                        col1.setWidth(w-1);
                        col1.setWidth(w+1);
                    }

Comment Story 4: Gabor and Peti
Gabor is new to the project. At some point during his learning curve, he has to fix quite a complex bit of code, a method of over 100 lines checking that some value fulfills all requirements before being submitted to the data store. It's quite a lot of tests, but still, he is not sure. Was everything tested? So in case someone comes up with another idea, he puts in the following comment in the code, at the end of the method:

    // ??? anything else?

Peti did read this comment, and found it funny. He actually felt the need to answer the question, and to show some positive and optimistic reaction to comfort Gabor that there would probably not be, but with the firm knowledge of experience that says "who knows...". He inserted the following comment:

    // Oh, no.

Comment Story 5: Baby come back
Sometimes, developers get bored, because the code they have to write does not present any challenge or fun. So they find way to have fun, and one way to do it is to put original comments in the code. That's probably what happened to Peti, who wrote some code which copies some value, use the original storage for some calculation, then restore the original value to its storage. In normal time, he would have put a comment like "Putting the original value back". But instead, he wrote the following:

  // Oh, baby come back

Comment Story 6: Peti talking to himself
It happens that comments are a mean of discussion between developpers, as it happened in Comment Story 4 between Gabor and Peti. I found such a comment spawned on three lines which seemed to be a dialog. But looking back into the version tree, it turned out that these three lines were written by the same person:

        // maybe only atm?
        // what do you think?
        // i'm not sure.

Probably still waiting for an answer...

Pascal Lover

 This article was originally posted on JRoller on June 11, 2004

I have a friend/colleague whose favorite language is Pascal. However, like half of my company, he had to learn Java, and of course had to use it as well. In his firt pieces of code, he tried to use some of the tricks he gathered from his Pascal experience. I think that most of us are doing the same. My first C program looked like Pascal, and my first Java program looked more like a direct translation from C++. It takes time to understand the philosophy of a language and use it the way it was meant to be used.

One of the trick he was using comes from the fact that Pascal is not efficient in String comparison. So my friend tried to avoid the following code:

if (myString.equals("value1"))     {       //value1     }     else if (myString.equals("value2"))     {       //value2     }

Instead, he rather used his own method, which was using a search of a substring within a String. Here is how it looks like:

switch (Util.whichOne(myString,"@value1@value2@"))      {      case 0:         //value1       break;      case 1:         //value2       break;      }

Seeing this code during my code review, I ran a small speed test, and of course the Pascal way was slower (by a factor of 7) than the simple condition list. So not only the philosophy is different between languages, but the possible optimisations too.

Not only between languages, but also between versions of the same language. In the past, the String's equal() method looked something like that:

public boolean equals(Object o)   {     if (!(o instanceof String)) return false;     return (compareTo((String)o) == 0);   }

Many programmers, including myself, were use to directly call the compareTo() method instead. However, in more recent versions of Java, the equals() method is an entity of its own, optimised for speed, and is faster on average than the compareTo() if what you are interested in is really the boolean return value. One of the optimisations is to check if the two Strings are of different size. In that case, equals() can directly return false while compareTo() still needs to calculate the difference between the two first non-equal characters.

All that comes back to the usual advice: if you really need to optimise something, check that what you are doing is really an optimisation.

The advice on using equals() for String is even more true today, when Java uses an inline C function to perform the operation.

Conditional Spaghetti

 This article was originally posted on JRoller on June 24, 2004

Often, having a lot of if/else structures one after the other in a code means that something is wrong with the design. It's what Kent Beck would call a Bad Smell. And usually, you can remove the conditions by using polymorphism. However, it is not always possible, or using inheritance would complicate the design too much.

Here, I want to speak about a certain kind of conditional spaghetti, where the list of if/else is quite long and the body is the same for each branch. This case can be viewed as a conversion between 2 values.

Here is a code that I've seen in one of the programs I had to review. It decides what icon to use for a given object type. There were something like 80 conditional branches.

if (getType().compareTo("PSTNGateway") == 0)     node.setIcon(new ImageIcon(       RCMGR.getImage("Image.Gateway")));   else if (getType().compareTo("PSTNNode") == 0)     node.setIcon(new ImageIcon(       RCMGR.getImage("Image. Node")));   else ...

My solution for this kind of code is to use a static HashMap for converting the object's name to its icon. Here is the HashMap declaration:

private static HashMap nodeTypes = new HashMap(50);   static {     nodeTypes.put("PSTNGateway", "Image.Gateway");     nodeTypes.put("PSTNNode", "Image.Node");     ...   }

Then, the 160 lines of conditional code becomes a simple two lines of code:

node.setIcon(new ImageIcon(RCMGR.getImage((String)     nodeTypes.get(getType()))));

Not only it is easier to read and maintain, but the 40 String comparisons on average become a hash code calculation and a couple of comparisons. The same technique can be applied to a Factory. From the same program, the code that was creating the nodes according to some parameter String looked like this:

if (cPar.getType().compareTo("BSC") == 0)     newNode = new TDBSC(cPar.getName());     else if (cPar.getType().compareTo("RNC") == 0)     newNode = new TDRNC(cPar.getName());     else ...

Now using the HashMap to convert between the node's name and the corresponding class:

private static HashMap nodeTypes = new HashMap(150);   static {     nodeTypes.put("BSC", TDBSC.class);     nodeTypes.put("RNC", TDRNC.class); ...   }

Class nodeClass = (Class)nodeTypes.get(cPar.getType());   Constructor init = nodeClass.getConstructor(new Class[] {String.class});   newNode = (TDTreeNode)init.newInstance(new Object[] {cPar.getName()} );

As a final note, I want to mention that the HashMap can be totally avoided by using naming convention. Then, the node class for example can be inferred with a simple formula:

Class nodeClass = Class.forName(PACKAGE_NAME + CLASS_PREFIX + nodeType);

This article was written in Java 1.4. Now, of course, using generics, you can avoid all those ugly castings. I used this pattern several times following this article. The gain in number of code lines is big, but it comes also with a notable gain in performance.