Hairshirt on backwards
In order to make big things easy for myself when programming, I do all manner of small things to make it harder. I call this Hairshirt Programming, and have been espousing its virtues since I first coined the phrase in Imperfect C++. Today I had it on backwards, and was caught out.I'm working on various command-line tools at the moment. When such programs take a number of flags/options, it is my usual practice to use a set of bit-flags to represent succinctly the various behavioural permutations.
One might test such flags as
if(flags & DO_THIS_WAY)
{
. . . // do it this way
}
It is my practice to do conditional expressions backwards (a Hairshirt Programming practice), in order to avoid the possibility of accidental assignment. Consider the error:
if(x = 1) // Oops!
when actually this was intended:
if(x == 1) // is x 1?
The way to avoid this is to reverse it, thus:
if(1 == x)
which the commutativity of the equals operator make equivalent to:
if(x == 1)
This has saved me many times, as the compiler has been forced to come to my rescue, and let me know that I'd expressed myself badly.
(Note: getting this wrong is possible in C and C++, not in C# or Java, whose syntax rules prevent it. Also, most C/C++ compilers will warn you about it if you turn up the warnings, which you should - another Hairshirt Programming recommendation.)
When testing flags in any more complex manner than that shown in the first example, I also tend to do the reversing (though in this case it's more out of consistency than need), as in:
if(0 == (DO_THIS_WAY & flags))
{
. . . // do it this way
}
Note that both the binary equality operator and the bitwise AND operator are reversed.
Never had a problem with this either. Until today.
I was working on a program and adding in support for a new feature, which in turn requires a new flag. Yet another Hairshirt Programming practice is to deliberately cause things that are a work-in-progress to be in an uncompilable state, that way the compiler tells you that you're not finished.
I'd just finished adding in the functionality that would respond to the new flag, and was about to define the new flag (and add the logic to detect it from the command-line).
if(0 == ( & flags))
As is my habit, I hit the compile button anyway, so that I could easily step around the code to the relevant parts (where compiler errors would undoubtedly ensue). I was surprised to see that there were no compile errors. What gives?
Well, look again at the expression above. flags is an int. So &flags is int*. As is (&flags). And, in C++, a pointer may be compared against 0. Yikes!
If I hadn't reversed the bitwise AND operands, or I was comparing against any other integral value than 0, I'd have had the compiler error I was expecting. It's subtle, to be sure, but nasty, since the expression is valid C++, not likely to precipitate any kind of warning, and will never be true!
Now I have another syntactic form to add to my automated code review settings. The irony is that this occurred in the source of one of my automated code review tools!

