Step on that Code

In Sociable Code I mentioned that an often overlooked consideration when writing code is the strain that it puts on the programmers who end up reading it. Difficult to read code tends to mash the wetware and end up causing more bugs and longer resolution times. One way to reduce that effort is to write code that would make control flow explicit when being stepped through with a debugger. Take a look at this bit of code. I came across it during a foray into a rather well known programming site. I have endeavoured to anonymise the code but the key piece has not lost it’s form.

bool isPerson = true;
char* personName = "D. Duck";
char* companyName = "Acme Co";
 
char* textEntered = "D. Duck";
 
if ( strcmp( isPerson ? personName
             : companyName, textEntered ) == 0 )
{
    cout << "Record Exists" << endl;
}

Notice the nested if statement? No? Look again.

isPerson ? personName
         : companyName, textEntered 

The conditional operator (short hand if statement) is in reality a junction in the flow of the code. It does one thing or the other depending on it’s first operand – in this case the boolean personName. It does not however lend itself to easy identification because it’s nested in a function call (strcmp) and can be stepped through with a debugger without that telltale indentation of the current line that tells the programmers brain that theres some flow of control happening there.

Now the hapless programmer who has to comes later on would probably see this bit of code for the first time when he’s seventeen levels into stack and wondering what to have for lunch. He’s probably not going to realise the short hand if is a possible control flow junction. While it saves some buffer space and it’s more terse it’s far less sociable to hide a switch like that inside a function call wrapped in another if statement. Unless you look very closely this block is going to look just like a hundred other if blocks that the programmer has already seen that contain no control flows whatsoever. He’s probably going to expect it to be the same and skip over not realising the importance of the block.

The reason for this is of course the wonderful story telling ability of the brain. It sees the block and it knows that blocks that look like that always have a check inside for something but that the control flow within the if test doesn’t branch itself. Even a careful programmer who has trained himself in the arcane arts of seeing the invisible might in a moment of weakness or a burst of caffeine induced hyperactivity skip over it. Here’s what the block of code would like like in a more pedestrian version of C++.

char* textToMatch = NULL;
 
if ( isPerson )
    textToMatch = personName;
else
    textToMatch = companyName;
 
if ( strcmp(textToMatch, textEntered) == 0 )
{
    cout << "Record Exists" << endl;
}

Stepping through this code makes it explicit that there is a decision being made at the two if blocks. A visual debugger would provide the programmer with instant feedback on which path the program takes and sometimes that distinction might just be the leaf under which the current bug is hiding.

Perhaps something to consider is that the ternary if statement itself is not the cause of the problem but rather it’s nested use. Consider the following case:

const double Charge = 100.0;
double chargeOnPerson = 0.0;
double chargeOnCompany = 0.0;
 
(isPerson ? chargeOnPerson : chargeOnCompany) =  Charge;

Here the left hand side of the = operator is a ternary if shorthand which assigns to either chargeOnPerson or chargeOnCompany depending on the value of isPerson. Notice that this example is more explicit as to the fact that there is a decision being made because it’s not nested in anything else. While this example is easier to recognise as a control flow one rather unattractive feature of it is that the left hand side argument in the = operation is no longer deterministic. This means that when this statement has been executed the programmer is left wondering which variable was changed as opposed to it’s more expressive kin shown below.

if (isPerson)
    chargeOnPerson = Charge;
else
    chargeOnCompany = Charge;

This is also the case for the ternary if statement appearing on the right hand side of the = operator.

double amountToCharge = (isPerson ? privateCharge : companyCharge); 

Here again the programmer has to check the value of isPerson to know which one of the two charges has been assigned to amountToCharge. This adds just a little bit of pain to the programmer debugging the code but is not as bad as the nested example shown at the beggining and so should be acceptable in most cases – particularly in those that require many ternary statements to set up configuration parameters and the like.

So be careful when writing code to ensure that important junctions of control flow visually jump out at programmers when they step through your code, doing so should provide with just a little bit more comfort for the those who come after.