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



10 comments:

Alistair McKinnell said...

I don't think that the style that you critique is motivated by optimization. Your post clearly showed that you don't get any performance benefit.

My understanding is that the style was adopted by some C programmers, myself included, to eliminate a whole class of bugs due to typing '=' when they meant to type '=='. That is, typing the assignment operator when they meant to type the equality operator.

By always putting the constant on the left hand side of any comparison the compiler will catch the typo.

A typical C/C++ if statement is to compare a pointer dereference to zero:

if (*p == 0) { /* Correct */

if (*p = 0) { /* Typo with no compiler error */

if (0 = *p) { /* Typo with a compiler error */

Now, the problem with this style is that it is unnatural to most people because it doesn't read like regular text.

Not only that, modern compilers, such as the Java compiler, are much better at catching this sort of typo than most C/C++ compilers were when I started writing C code. So the benefit of the style is very much diminished.

By the way, I find the style of constant on the left very readable :)

Alistair

gogle said...
This comment has been removed by a blog administrator.
tenax_technologies said...
This comment has been removed by a blog administrator.
aisha said...
This comment has been removed by a blog administrator.
Nick said...

Thanks for post such as very useful and very lovely post.....

Software Product Development Services: - Ampere offers full cycle custom software programming services, from product idea, offshore software development to outsourcing support and enhancement.

sheena said...

I enjoyed your post. It’s a lot like college – we should absorb everything we can but ultimately you need to take what you’ve learned and apply it.
Chevy W6 Turbocharger

Android app developer said...

This is one of the knowledgeable post.I like your blog tips.This is one of the useful post.

website designing development company chennai india said...

I have read this post. collection of post is a nice one ..that am doing website designing company chennai india and website development company chennai india. That I will inform about your post to my friends and all the best for your future posts..

smartsuite said...

I believe Web time sheet software makes the complete employee time clock tracking task easier. Its easy to update, approve and maintain the time sheets in no time.Time Attendance Software

Wizard Infoways said...

WIPL is a global leader in providing software solutions and it is one among the best web development company in India.

For more info : BI Solutions in Noida