CodeSOD: Not a Not Bad Approach

In terms of elegance, I think the bitmask has a unique beauty. The compactness of your expression, the simple power of bitwise operators, and the way you can see the underlying implementation of numbers laid bare just speaks to me. Of course, bitmasks can be a bit opaque, and you may have to spend some time thinking about what foo &= 0xFF0000 is actually doing, but there's also something alluring about it.

Of course, bitmasks are surprisingly hard. For example, let's look at some code submitted anonymously. This code is meant to run on a line of coin-operated dryers. Depending on the particular install, how many coins a patron puts in, what modes have been enabled by the owner, and so on, different "extra" features might be disabled or not disabled.

Since bitmasks are hard, we're not going to use one. Instead, let's have a pile of constants, like so:

#define DRYER_EXTRA_1 1#define DRYER_EXTRA_2 2#define DRYER_EXTRA_3 3#define DRYER_EXTRA_4 4#define DRYER_NOT_EXTRA_1 6#define DRYER_NOT_EXTRA_2 7#define DRYER_NOT_EXTRA_3 8#define DRYER_NOT_EXTRA_4 9

Now, how might we use those constants? Why, we're going to use constructs like this:

if(supportedExtra == DRYER_EXTRA_1){ notSupportedExtrasList[extra1] = DRYER_NOT_EXTRA_1;}else{ notSupportedExtrasList[extra1] = DRYER_EXTRA_1;}

A double negative is not a not bad idea. If this dryer is configured to support DRYER_EXTRA_1, then we'll add DRYER_NOT_EXTRA_1 to the list of unsupported extras. There's a variation on this code for every possible extra, and they all use the double-negative pattern.

As extras are being selected, however, we need to check against things like if there's enough money in the machine. Thus, we see patterns like:

if(isEnoughMoney == FALSE){ selectedExtra &= ~DRYER_EXTRA_1; //so this is a bit mask now?}//...some more lines laterif(selectedExtra == DRYER_EXTRA_1){ //we're back to treating it as integer?}//...some more lines laterif((selectedExtra & DRYER_EXTRA_1) == DRYER_EXTRA_1)

Our developer here gets confused about the nature of the constants, swinging from manipulating it as bitmasks (which won't work, as the constants aren't powers of two), then back as a regular number (which could work, but maybe isn't the best way to design this), and back to bitmasks again.

You may be shocked to learn that this entire module doesn't work, and our submitter has been called in to junk it and rewrite it from scratch. They'll be using bitmasks.

[Advertisement] ProGet supports your applications, Docker containers, and third-party packages, allowing you to enforce quality standards across all components. Download and see how!