Monday, February 23, 2009

Make it Maintainable and Readable First

You're probably aware of the Donald Knuth quote "Premature optimization is the root of all evil (or at least most of it) in programming.". I reviewed some code recently that reminded me of that quote and why making code readable and maintainable is the first order of business. The code I reviewed reversed the usual order for checking for a null reference to -

if (null != instance) { ...}

I don't like this code because it uses a non-standard idiom and forces me to stop and read the code from right to left to understand it. I've run across this idiom once before and the best reason I have gotten for using it is that it is supposed to be faster. Well, I can't imagine why it would be faster and even it if was there are certainly better performance optimizations that don't affect code readability. Being curious I decided to test the this optimization and wrote a little java test code.

public class NullTest {
public static long testNullFirstAgainstNonNull(int count, boolean display) {
long start = System.currentTimeMillis();
Object obj = getObject();
int x = 0;

for (int i = 0; i < count; i++) {
if (null == obj) {
x++;
}
}

long end = System.currentTimeMillis();
if (display) System.out.println("if (null == [non-null]) -- " + (end - start));
return x;
}

public static int testNullFirstAgainstNull(int count, boolean display) {
long start = System.currentTimeMillis();
Object obj = getNullObject();
int x = 0;

for (int i = 0; i < count; i++) {
if (null != obj) {
x++;
}
}

long end = System.currentTimeMillis();
if (display) System.out.println("if (null != [null]) -- " + (end - start));
return x;
}

public static int testNullLastAgainstNonNull(int count, boolean display) {
long start = System.currentTimeMillis();
Object obj = getObject();
int x = 0;

for (int i = 0; i < count; i++) {
if (obj == null) {
x++;
}
}

long end = System.currentTimeMillis();
if (display) System.out.println("if ([non-null] == null) -- " + (end - start));
return x;
}

public static int testNullLastAgainstNull(int count, boolean display) {
long start = System.currentTimeMillis();
Object obj = getNullObject();
int x = 0;

for (int i = 0; i < count; i++) {
if (obj != null) {
x++;
}
}

long end = System.currentTimeMillis();
if (display) System.out.println("if ([null] != null) -- " + (end - start));
return x;
}


private static Object getObject() {
return new Object();
}

private static Object getNullObject() {
return null;
}

public static void main(String[] args) {
int count = 100000;
testNullFirstAgainstNonNull(count, false);
testNullFirstAgainstNull(count, false);
testNullLastAgainstNonNull(count, false);
testNullLastAgainstNull(count, false);
testNullFirstAgainstNonNull(count, false);
testNullFirstAgainstNull(count, false);
testNullLastAgainstNonNull(count, false);
testNullLastAgainstNull(count, false);

count = 100000000;
System.out.println(" Results from " + count + " executions.");
System.out.println("============================================================");
testNullFirstAgainstNonNull(count, true);
testNullFirstAgainstNull(count, true);
testNullLastAgainstNonNull(count, true);
testNullLastAgainstNull(count, true);
System.out.println();
testNullFirstAgainstNonNull(count, true);
testNullFirstAgainstNull(count, true);
testNullLastAgainstNonNull(count, true);
testNullLastAgainstNull(count, true);
System.out.println();
testNullFirstAgainstNonNull(count, true);
testNullFirstAgainstNull(count, true);
testNullLastAgainstNonNull(count, true);
testNullLastAgainstNull(count, true);
System.out.println();
testNullFirstAgainstNonNull(count, true);
testNullFirstAgainstNull(count, true);
testNullLastAgainstNonNull(count, true);
testNullLastAgainstNull(count, true);
System.out.println();
testNullFirstAgainstNonNull(count, true);
testNullFirstAgainstNull(count, true);
testNullLastAgainstNonNull(count, true);
testNullLastAgainstNull(count, true);
System.out.println();
}
}

The test tries to prime execution and warmup the JVM by running each test before displaying any results. The usual caveats about micro-benchmarks, etc. apply to the results below. I ran this under to Sun JVMs, 1.4 and 6, with similar results that show no performance benefits and just reinforce for me the importance of writing code that is readable, maintainable and understandable.

$ /c/j2sdk1.4.2_17/bin/java -version
java version "1.4.2_17"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_17-b06)
Java HotSpot(TM) Client VM (build 1.4.2_17-b06, mixed mode)

$ /c/j2sdk1.4.2_17/bin/java NullTest
Results from 100000000 executions.
============================================================
if (null == [non-null]) -- 141
if (null != [null]) -- 156
if ([non-null] == null) -- 141
if ([null] != null) -- 172

if (null == [non-null]) -- 141
if (null != [null]) -- 156
if ([non-null] == null) -- 140
if ([null] != null) -- 172

if (null == [non-null]) -- 141
if (null != [null]) -- 172
if ([non-null] == null) -- 140
if ([null] != null) -- 157

if (null == [non-null]) -- 140
if (null != [null]) -- 172
if ([non-null] == null) -- 141
if ([null] != null) -- 156

if (null == [non-null]) -- 141
if (null != [null]) -- 172
if ([non-null] == null) -- 140
if ([null] != null) -- 172


$ java -version
java version "1.6.0_07"
Java(TM) SE Runtime Environment (build 1.6.0_07-b06)
Java HotSpot(TM) Client VM (build 10.0-b23, mixed mode, sharing)

$ java NullTest
Results from 100000000 executions.
============================================================
if (null == [non-null]) -- 62
if (null != [null]) -- 63
if ([non-null] == null) -- 62
if ([null] != null) -- 63

if (null == [non-null]) -- 62
if (null != [null]) -- 78
if ([non-null] == null) -- 63
if ([null] != null) -- 62

if (null == [non-null]) -- 63
if (null != [null]) -- 62
if ([non-null] == null) -- 63
if ([null] != null) -- 62

if (null == [non-null]) -- 63
if (null != [null]) -- 62
if ([non-null] == null) -- 63
if ([null] != null) -- 62

if (null == [non-null]) -- 63
if (null != [null]) -- 62
if ([non-null] == null) -- 63
if ([null] != null) -- 62