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



8 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

Unknown said...
This comment has been removed by a blog administrator.
Elmira said...
This comment has been removed by a blog administrator.
aisha said...
This comment has been removed by a blog administrator.
Android app developer said...

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

Zinavo-Web Design | Web Development | SEO | Mobile Apps | ERP/CRM said...

Its very amazing article to give a good quality content.Website Development Company in Bangalore | Website Designing Company in Bangalore | Website Design Company in Bangalore

anjali said...

Convert speech to text in Python
PHP set timezone
Convert stdclass object to array PHP
Remove duplicates from array PHP
Remove empty values from array PHP
Remove duplicates from array Javascript
PHP capitalize first letter
PHP get array key
Age calculator in PHP

Ace Infoway said...

This was a great post. Finding the time and genuine effort to make a great article… but what can I say… Thanks for providing these details. Love from Ace Infoway.