Avoid destructive updates

This is an excerpt from a document I’ve been working on for some time now.

A destructive update to a variable or field means assigning a new value to it even though it already has a value assigned. Let’s have a look at the anamnesis of a small piece of code which I found in the VCS history.

ValueType x = null;if (yType.isSetType()) {
    x = yType.getValue();
} else {
    x = yType.addNewValue();
}
x.doSomething();

x is first null, then something else is assigned depending on some condition. The next programmer introduced this:

ValueType x = null;
if (yType.isSetType()) {
    x = yType.getValue();
} else {
    if (unsetFlag) {        yType.unsetValue();
        x = yType.addNewValue();
    }
}
x.doSomething();

Now it still compiles, but a NullPointerException will occur at runtime – sometimes. That’s bad. The principle “Fail early, fail hard” is best applied by failing at compile time with a compile time error. The unnecessary destructive update prevented that. Better start with:

final ValueType x;if (yType.isSetType()) {
    x = yType.getValue();
} else {
    x = yType.addNewValue();
}
x.doSomething();

Then after

final ValueType x;
if (yType.isSetType()) {
    x = yType.getValue();
} else {
    if (unsetFlag) {
        yType.unsetValue();
        x = yType.addNewValue();
    }
}
x.doSomething();

it doesn’t compile anymore (can you see why?). The compiler caught the problem before it was too late.

The programmer is forced to take action and will change to what he really needed instead:

final ValueType x;
if (yType.isSetType()) {
    x = yType.getValue();
} else {
    if (unsetFlag) {
        yType.unsetValue();
    }
    x = yType.addNewValue();
}
x.doSomething();

By the way, Eclipse doesn’t even show a warning for the code that causes the NPE. Especially when working with bad code, the inspection features of Intellij Idea are mandatory. When working with professionals, an editor and a compiler will do.

Another example:

int x = 7;
if (b) x = 8;
if (c) x = 9;

if without else is usually bad, and destructive updates are used in this example as well. It’s not an example from the real world like the one above that caused a concrete problem, but still should be rewritten to this:

final int x;
if (b) x = 8;
else if (c) x = 9;
else x = 7;

Destructive updates are one of the biggest problems in old software monoliths. Just by declaring everything final, one will automatically move towards a functional programming style and avoid many problems. “Everything” means everything: parameters, Exceptions in catch blocks, fields, local variables. I noticed something interesting: Programmers who avoid destructive updates themselves are much, much better at detecting problems related to destructive updates in other people’s code. Destructive updates are not something that bad programmers recognise as a phenomenon, they just do it without being aware of the consequences.

Now a dispute between communities that recognise this principle is whether it should be followed absolutely in languages like Java or not. There are many situations in which it would be clumsy and inconvenient to absolutely avoid destructive updates in Java due to language limitations. Some programmers compromise in that case, others don’t.

Leave a Reply

You must be logged in to post a comment.