Tuesday, 26 April 2011

Good Programming Practice - avoiding the if statement

Those of us who have programmed for a while are fairly familiar with and comfortable using the if statement in code but the if statement if mis-used can be both confusing and a source of latent defects, that is defects which are not realised for a while and then jump out and bite you. What is the distinction between a good and a bad if?
A good if statement should always be valid whatever happens to the system so a method which creates a type of class to manage a port might look like:

if ( typeToCreate == "TCP" ) port = new TCPPort();
else if ( typeToCreate == "HTTP" ) port = new HTTPPort();

If you think about it, although we might decide to use a different port in the future, the logic always holds true that if the type is TCP, we will create a TCP port. If we need to expand the functionality of the port by extending its interface, we can do so without 1) touching the if statement and 2) invalidating the logic of the if statement. Ideally, we would then have an else statement which might throw an exception to ensure we are using a valid port type.
Now consider the logic below in a classic if statement:

LoadMessage();
if ( message.SubString(0,1) == "S" ) DoSomethingForStringMessage();
// Carry on with processing

Can you see what the problem is here? There is an assumption buried deep beneath the code that says a message which is a string ALWAYS has an S as the first character and a message which does NOT have an S is NEVER a string. This might seem perfectly reasonable and correct but what happens when somebody decides in 3 years time that X represents an extended string. These new messages will fail the logic test but possibly in a way that is not obvious. This is a very easy to create defect which might well pop its head up at the customer site!
I am not a Puritan but I believe in making code as reliable as possible. We would not accept an aircraft designer thinking something is a quick hack but yet we allow it in software because perhaps we don't give it the respect and effort that should be given to it. Of course, it would be unfair to bring this issue up without telling you about alternatives. Ultimately there are three things that can really help you 1) The compiler is your friend - use constructs that the compiler can check 2) OO design allows you to embed logic into classes and 3) Do not repeat yourself. If you need to ask the same question in multiple places, you are asking for one or more of them to fall out of sync.
Here is an example of using the compiler and OO to help you code logic strongly:

LoadMessage();
message.PreProcess();
// Carry on

Here we introduce a class to represent the message (it might only be one class to begin with) and we create a method that is fairly generic and allows us to preprocess the message. In our case, we probably want a class to represent a normal message and one that represents string messages (which could inherit from the normal one). In this case, the string message PreProcess allows us to do whatever it is we needed to do inside this message with NO logical branching. Adding new messages with inheritance allows us to force people to consider the various methods that are available by making them abstract/pure virtual.
This leads to the 3rd principle which says in one place, and one place only, I will use some sort of data loader to decide which class to load for any new messages I create, this follows the safe method that the logic is always true since by definition, if I add a new message, I either have to extend or modify the logic in a single place to load my new handler class and this will remove the chance of a bug:

if ( message.SubString(0,1) == "S" ) CreateStringMessage();
else if (message.SubString(0,1) == "N" ) CreateNumericMessage();
else throw new NotImplementedException("No handler for message type");

If I add a new message which might also begin with S, I might need to do this:

if ( message.SubString(0,1) == "ST" ) CreateStringMessage();
else if (message.SubString(0,1) == "N" ) CreateNumericMessage();
else if ( message.SubString(0,1) == "S" ) CreateNewMessage();
else throw new NotImplementedException("No handler for message type");

The important thing being that the logic is in a single place and not buried somewhere down in a method. Using the exception mechanism for defensive programming is then an easy and quick way to fall over during testing if something has been forgotten.
Post a Comment