Coding Styles – A Fine Line Between Clever and Stupid

I’ve been thinking a lot about coding style recently. I must admit that I have probably always tended to code in a way that would be considered to be verbose by many programmers. Although I learnt to program by myself, I benefited from an extensive professional programming course in PL/I (which doesn’t date me too much, but it does date the machines I was working with!)

PL/I, as I recall, was a wonderful language that allowed you to do all sorts of things that are now forbidden in a type-safe world. For example, you could create record types which, similar to C structs, could be used to overlay an area of memory and then you could read or write to the memory through the typed record – which was fixed-length in those days, of course). But there were inevitably certain horrid things about coding in a mainframe environment; we had to number our record elements (essentially hard-coding hierarchies) amongst many other things. The result was that where I worked, coding style was relatively strictly enforced… but never really to optimise clock cycles or memory – more towards readability. Despite being in a mainframe environment, we had well passed the time where performance was really an issue for the basic number-crunching that we were doing. The point is, in my early programming days, I never had the concerns that seem to be more common amongst those who started in Unix and C.

I am almost sure that I have, at one time or another, made the mistake of testing a boolean expression in an ‘if – else’ statement when a simple assignment could have been used. For example, here’s some wasteful code:

if (booleanValue)
{
    someValue1 = true;
    someValue2 = false;
}
else
{
    someValue1 = false;
    someValue2 = true;
}

You’ll possibly have already seen the simple optimisation that could be made to this code:

someValue1 = booleanValue;
someValue2 = !booleanValue;

With well named variables, we might even achieve some reasonably readable self-commenting code:

okButton.Enabled = validationPassed;
validateButton.Enabled = !validationPassed;

But in practice, I am not so sure that these boolean comparison / assignments are that common – at least in isolation. Perhaps when a certain condition is met we might also need to set a label to a particular string value, or any number of ‘non boolean’ alternatives.

In which case, which of the following code fragments is better?

if (validationPassed)
{
    infoLabel.Text = "Please click OK to complete";
}
else
{
    infoLabel.Text = "Sorry there was a problem...";
}

okButton.Enabled = validationPassed;
validateButton.Enabled = !validationPassed;

Or…

if (validationPassed)
{
    okButton.Enabled = true;
    validateButton.Enabled = false;
    infoLabel.Text = "Please click OK to complete";
}
else
{
    okButton.Enabled = false;
    validateButton.Enabled = true;
    infoLabel.Text = "Sorry there was a problem...";
}

Personally, I think that the latter is generally more readable, as it places the response to the condition ‘is validation passed’ in one of two blocks below the if or endif. You are not excused from looking at surrounding code to see what else is happening, but our response to the condition in this example has been concentrated into two easy-to-compare sections.

I’m sure that this comes down to personal taste in a great many instances.

Whichever way, the ‘wasteful’ code in the first example may not have been so wasteful after all; sure, it could have been written more compactly – but it could be seen as concentrating the response to the condition and perhaps also, preparation for a future spec-change! You can’t really prepare for the unknown, but you can surely be reasonably certain that the future will not necessarily fit nice and neatly into a tidy little pattern that you realised would save you a few lines of code?

Leave a Reply

Your email address will not be published. Required fields are marked *