Monday, June 8, 2009

Most if-statements are evil

I’ve really come to loath if-statements. It developed over time. At the very beginning they are great as they give you the power to put some logic into you application. Usually a small program in one of your first programming classes. Well, over time I realized the hard way that they clutter your code and make it harder to read and hence understand and maintain. In OO languages you have far more powerful ways to put logic into your application without using if’s. However, this requires a good skill in programming and lots of practice. I’ve come to realize that you can identify a good programmer by her if-avoidance skill. Well, after some time in acceptance but dislike I now try to avoid them at any price and so should you. By the way, I consider switch-statements to be nested if’s.

Frederick Brooks coined once the terms ‘Essential Complexity’ and ‘Accidental Complexity’. The first describes the real complexity which comes from the domain and the problem to solve. The second one is manmade i.e. it is the complexity we get into by our own making like which OS, which language, which framework, … we choose. Using this point of view, there are essential if-statements and accidental if-statements. The accidental ones, what a fitting name, are the ones which need to be eliminated.

Essential if’s
There are things you need to check. Can I save the file, is there enough free disk space.

if (diskHasEngoughFreeSpace()) {
document.save();
}

Those decision are driven from external forces and are out of our control and so we need to check them every time we save.

Accidental if’s
Those buggers creep up everywhere in you code.

if (date == null) {
date = new Date(); // this is called defensive programming, one of the worst things you can do
}

or

if (date != null) {
uiWidget.setText(date.toString());
} else {
uiWidget.setText(“”);
}

Before you get creative in finding a workaround you should analyze whether date actually can be null. If yes, how? Is it a field of the class, a parameter of the method, a return value from a called method. All these questions and their answers provide you with a multitude of options.

/**
* @param date of the document to be printed; must not be null
*/
public void printHeader(Date date) {
// Usually I put those checks into a utility class and a checkParam method.
// i.e. Check.checkNotNull(date, “date”);
if (date == null) throw new IllegalArgumentException(“invalid parameter: date cannot be null. Good bye!”);

// once we are here we know that all parameter are valid
date.doSomething();
…
}

It is a good practice to have an object to be in well pre-defined state after creation.

class DocumentPrinter {
private Date date; // this is the same as private Date date = null;
…
}

It is better to do it this way

class DocumentPrinter {
private Date date = Date();
// if this is not sufficient then have  the date passed in as parameter with the constructor and make the default constructor private
…
}

or even better use the Null-Object pattern

class Document Printer {
private Date date = new NullDate();
…
}

class NullDate extends Date {
public void toString() {
return “not set”;
}

From now on you can use date anywhere in your class without the need to check for null.
Now lets look at a return value from a called method. First I would study the documentation of the method and see whether the method actually can return null objects. In important cases I would even go so far as to quickly write a bunch of tests. With those tests you have clear documentation if the method can return null and under which conditions.

public void doSometing() {
…
Date date = getDateOfTransaction(transaction);
…
}

Knowing, that date might be null we need to see how the variable is being used subsequently. If it is being used for some simple display purposes we can wrap the method and return a NullDate in case the return value is null. If there is some more serious work with the object down the road then ‘Houston we have a problem’! Creating a fake date will not work and invite some big risks. Bypassing the date accesses even more so:

…
doThis();
doThat();

if (date != null) {
rebate = calculateRebate(date);
bookTransaction(rebate);
}
…

Sure, we could move the bookTransaction-method out of the block and guarantee that it will be always booked. But, assuming that we are dealing with a preferred customer who always gets a 10% discount. In this case we will get bunch of calls to our customer service folks which would have to deal with an upset customer. We don’t want that. Depending on whether you have access to the source code of used method you can go in fix the problem if not then you have a problem which you should encapsulate. The encapsulated problem offers a clear separation between working and broken smelly workaround code. Also it adds a lot the readability of the source code and you would have only one place to go and do the fix.

But now let’s look at another piece of code. Assuming we have so transmit data via a selectable protocol. The transmission must be encrypted under specific circumstances. So the code could look like this

public void send(String text, String transmission, String encryption) {
String sendText = “”;

if (encryption.equals(“RSA128”)) {
encryption = new RSA128();
sendText = encryption.encrypt(text);
} else if (encryption.equals(“RSA1024”)) {
encryption = new RSA128();
sendText = encryption.encrypt(text);
} else { // no encryption
sendText = text;
}

if (transimission.equals(“ftp”)) {
FTPTransmitter transmitter = new FTPTransmitter();
transmitter.transmit(sendText);
} else if (transmission.equals(“ssh”)) {
SSHTransmitter transmitter = new SSHTransmitter();
transmitter.transmit(sendText);
}
}

How about this

public void send(String text, String transmission, String encryption) {
Transmitter transmitter = Factory.createTransmitter(transmission, encryption);

Transmitter.transmit(text);
}


First, we extracted a Transmitter interface or if feasible an abstract class. The transmitter internally uses a strategy pattern to encrypt the text. The encryption is injected during construction time of the Transmitter. Again, there is an abstraction for the encryption. The factory could be a own creation or you could use a framework like Spring.

class Factory {
public static Transmitter createTransmitter(String transmission, String encryption) {
Encryption encryptionObj = createEncryption(encryption);
return createTransmitter(transmission, encryptionObbj);
}

public static Transmitter createTransmitter(String transmission, Encryption encryption) {
Transmitter transmitter = createTransmitter(transmission);

transmitter.setEncryption(encryption);
return transmitter;
}
…
}

Sure, if you were following correctly I would expect a ‘but now we have the if-statements in the conveniently not shown createEnryption and createTransmitter method’. Yes, your are right. However, I consider those if-statements at this location to be essential. Also, if you use Spring the condition checking will be externalized into the bean descriptor file.
Also, since the factory is driven by Strings, these could easily come from the UI. In that regard the UI would drive the behaviour without adding any conditional checking and would be easily extendable to other means of transmission and encryption without the need to modify it. And yes, the code will much easier to test. MockObjects anyone ;)

PS If anyone can give a me hint to how show source code in a nice and readable using Blogger. This would be highly appreciated.

14 comments:

Chris Burnley said...

Awesome post, totally agree. I've been saying this for years but people just think I'm mental :P

Marco Valtas said...

Paulo, I totally agree with your post. In OO there´s ways to avoid if´s statements but sometimes you have to do it. In fact, when you mention the defensive programming, one of the worst problems in programming (in my opinion), I remembered a problem recently in my application.

For a JPA Query object, which should return a list, the API makes no statement if there´s no result the list returned should be empty or a null is returned. the Hibernate implementation returns null, which made me check whatever the result was null and create a empty list. In this case I realized that it´s impossible for a developer to know all the behavior of a complex system, so defensive programming has to take place somewhere, but this creates another problem. A huge code base mixed with business logic and care to not throw a unexpected exception and blow to system.

Another problem was with JAAS, it´s not clear whether calling getPrincipal() without a security domain will return a null or a EJBAccessException before the application server even reach the method. It turns out that neither are true, calling getPrincipal() will rise a exception in JBoss in such circumstances.

Well, the moral is, every system will fail (not gracefully) sometime because it´s practically impossible to check every API variation in a complex environment.

For source code in blogger I use this http://heisencoder.net/2009/01/adding-syntax-highlighting-to-blogger.html

Best.

rnaufal said...

Good post, totally agree with it. For displaying source code in a nice and readable way I use JEdit with Code2HTML plugin.

You can check this at my blog here

Philip Schwarz said...

See also The Anti-If Campaign

aiai said...

For a pair of sleek gloves that would look great with your suit, pick up the Ross Gloves Italian Sheepskin Gloves with Cashmere lining. These gloves are perfect for those who live in metropolitan areas
spyder skiwhere the temperatures drop in the winter because these gloves are lined with extra warm and soft cashmere for that added layer of spyderwarmth and insulation. The premium Italian lambskin will mold to your hands in a close fit and the slim lines of this glove are convenient enough to slip into your pockets when not neededspyder jacket.

For a great pair of gloves that will keep up with you on the slopes even spyder ski wearin the harshest of temperatures, pick up the Burton' Gore-Tex Under Gloves for $64.95. The Thermacore insulation on these ski jacketgloves will keep your fingers warm even when the freezing wind and slate beats down on you. spyderThe DryRide Ultrashell combined with the Gore-Tex Insert ensures that nothing will get your fingers wet. spyderThe hidden heater and vent pocket stashes extra warmth on the cold days and dumps spyder jacketsexcess heat in the spring. spyder ski wearIf you are worried about your goggles getting steamed up, these gloves help solve that problem with the soft chamois goggle wipe which will keep your vision clear.

jessica said...

COMPAQ Presario 2160 Series Laptop Battery
COMPAQ Presario 2161 Series Laptop Battery
COMPAQ Presario 2162 Series Laptop Battery
COMPAQ Presario 2163 Series Laptop Battery
COMPAQ Presario 2164 Series Laptop Battery
COMPAQ Presario 2165 Series Laptop Battery
COMPAQ Presario 2166 Series Laptop Battery
COMPAQ Presario 2167 Series Laptop Battery
COMPAQ Presario 2168 Series Laptop Battery

贝贝 said...

The Tax Return Crack-Up<3>
Granted, there are usuallyMicrosoft Office 2010write-ups when presidential contenders make their tax returns available, but the coverage falls far short of the Office 2010
full court press (pardon the pun) that the Clintons have received. What's Microsoft Office 2007different now?Office 2007One possibility is that most upper middle class Democrats, and therefore most Microsoft OfficeOffice 2007 keyeditors and reporters of our nation's big papers as well as Office 2007 downloadtelevision producers, are Obama supporters who think that Hillary should hurry up Office 2007 Professionaland drop out of the race already.Microsoft outlook
Microsoft outlook 2010Whom elite liberals are pulling for really does shape political coverage in ways

sally21c said...

The factors you need to be aware of zigtech, the two most important is the durability and Comfort of the zigtech reebok. Paying for high price for reebok from a reputable brand is not a bad idea As They must have used better quality reebok shoes that will withstand The Elements is zigtech shoes. A well manufactured jacket Will show at Signs of Reebok Easytoneeasytone and easytone shoes should be easy to use.

sally21c said...

Moncler men and Moncler women clothing-Moncler online here,The Moncler coats simple line of the Moncler outlet that you adopt——buy a Moncler veat that it has gained.

sally21c said...

We sell Moncler,Moncler jackets in different styles at Moncler outlet shop.moncler is leading a trend, moncler jacken and Moncler coats you can get online.

GuildWars2Items said...

The past is gone and static. Nothing we can do will change it Runescape Gold, The past is gone and static. Nothing we can do will change it Buy Runescape Gold Sale, Everything we do will affect it RS Gold.

GuildWars2Items said...

Would you write to please D3 Gold just yourself? Or others? Or yourself by writing for others Cheap D3 Gold? It takes strength to do what must be done when the work is unpleasant and uncomfortable D3 Gold Sale, And of what would you write: Of love? Hate? Fun? Misery? Life? Death?Nothing Everything?

dte said...

It's great to be great, but it's greater to be human Sell Runescape Gold, if I could rearrange the alphabet, I'd put buy guild wars 2 gold Y and I together guild wars 2 gold buy, A lifetime of happiness! No man alive could bear it; it would be hell on earth .

dte said...

Life is not measured by the number of breaths we take scarlet blade gold, but by the moments that take our breath away scarlet blade gold. I have a simple philosophy: Fill what's empty scarlet blade gold.