Blog-Archiv

Dienstag, 26. Februar 2008

Things Are Changing

I always loved to look at Mandelbrot viewers that let me dive into endless worlds with bays that end in a lot of other bays. Sometimes I wondered if reality is like that, an endless splitting of ends into other ends. I mean, maybe it is only my perception of reality that changes all the time? Or is it me that changes? Anyway, I do not believe very much in the sentence "Change yourself and the world will change". I have been driving so long with my bicycle through this city, and still car drivers are angry at me. I changed, but the world stayed the same.

However, lets look at another kind of change. How can Java source change when you remove code smells? Look at following classes, containing the code smell called "DontCallOverridableMethodsInConstructor":

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
  class Flag
  {
    private boolean value;

    public Flag() {
      setValue(true);
    }

    public void setValue(boolean value) {
      this.value = value;
    }
  }
1
2
3
4
5
6
7
8
9
  class NamedFlag extends Flag
  {
    private String name = "undefined";

    public void setValue(boolean value) {
      super.setValue(value);
      this.name = "Flag is " + value;
    }
  }
1
2
    NamedFlag flag = new NamedFlag();
    System.err.println("flag.name: " + flag.name);

The smell causes the name to have an unexpected value, the program outputs:

flag.name: undefined

First the super-constructor of class Flag works, calling setValue(), control goes to NamedFlag.setFlag(), which initializes a field of an instance that is not yet constructed, so the following construction initializes it (correctly?) to "undefined".

This is a rather frequent case, it is normal to call some setter from the constructor. Every public setter method called from constructor ought to be final to avoid the code smell. But we need it overrideable, so we remove the smell in another way.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
  public class Flag
  {
    public static Flag newFlag() {
      Flag flag = new Flag();
      flag.init();
      return flag;
    }
    
    private boolean value;

    private Flag() {
    }

    protected void init() {
      setValue(true);
    }

    public void setValue(boolean value) {
      this.value = value;
    }
  }
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
  class NamedFlag extends Flag
  {
    public static NamedFlag newNamedFlag() {
      NamedFlag flag = new NamedFlag();
      flag.init();
      return flag;
    }
    
    private String name = "undefined";

    private NamedFlag() {
    }
    
    public void setValue(boolean value) {
      super.setValue(value);
      this.name = "Flag is " + value;
    }
  }
1
2
    NamedFlag flag = NamedFlag.newNamedFlag();
    System.err.println("flag.name: "+flag.name);

Now the programm outputs the expected result:

flag.name: Flag is true

By doing the construction in a static factory before setting values into the object, we achieve a valid name. The price is a factory for every subclass of Flag. And we had to avoid code duplication by introducing a protected init() method where we could call some other initializing setters.

Things have changed, the code smell disappeared, but we needed more source. Technical code that is quite boring to read, and someone might wonder what it is good for.
We could say

Why does Java such things? Wouldn't it be better when any constructor just calls the nearest setter, the Flag constructor calling Flag.setValue(), the NamedFlag constructor calling NamedFlag.setValue()?

Good idea, but this would break Java language specification and kill a lot of existing Java applications. Things are changing because other things can not change, because they are already in use and well-known.

Nevertheless the code smell is legal. I committed this initialization mistake several times, and surely I am not alone. Besides, this error is not so easy to find.

Do we need a new programming language? One with built-in Factories, and with an overridable Object.init() method, analoguous to finalize()? One that does not insist on OO paradigmas but concentrates on the essence and is really good readable?

Often it has been stated that Java is so repetitive, a line like

Flag flag = new Flag();

is quite boring to read. And it sounds so silly. Only developers understand the deeper sense, that memory gets allocated for a new object instance, and it gets garbage collected as soon as there is no more reference to that object. Still a lot of technical thoughts are in it, even with Java.

But it is too early for complaining, we are not done! I just detected another code smell: "CallSuper". This says that every overridable method should provide another overridable method for subclasses, so that it is not up to the developer to remember that super.setFlag() needs to be called. Fixing it, the source would look like the following:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
  class Flag
  {
    ....

    public void setValue(boolean value) {
      this.value = value;
      setValueSideEffects(value);
    }

    protected void setValueSideEffects(boolean value) {
    }
  }
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
  class NamedFlag extends Flag
  {
    public static NamedFlag newNamedFlag() {
      NamedFlag flag = new NamedFlag();
      flag.init();
      return flag;
    }
    
    private String name = "undefined";

    private NamedFlag() {
    }
    
    protected void setValueSideEffects(boolean value) {
      name = "Flag is "+value;
    }
  }

Consider having another subclass IdentitfiedNamedFlag extending NamedFlag. We would have to introduce one protected setValueSideEffectsXxx() method per subclass to avoid the CallSuper code smell!

Things are changing. The more source we write, the more errors it will have. More source, more code smells. The closer we look the more source we get out. And it is not nice for source maintainers that try to reduce all that stuff to its really needed amount.

I have no opinion on all that changes. I just try to ride on them, like a surfer on the waves. This is the only concept I found to deal with such things.