## Daily WTF

I often read the “Daily WTF” because there’s something satisfying about seeing other people’s bad code. “See? Our code isn’t as bad as all this!” It’s not as fun when you find “Daily WTF” moments in your codebase.

Today, one of my coworkers asked about a piece of code that wasn’t giving the expected results. It’s a part of some really old code that would be done differently given the time to rewrite it. Any way, there are a series of if/elsif/else clauses that check various things. One of those things is to validate some data against a known constraint. This particular section is supposed to validate that the given data falls within the range of a valid UINT (UINT8, UINT16, etc).

``````elsif ( \$constraint =~ /^UINT(\d+)/ )
{
\$start = 0;
\$end = ( 0x1 << ( \$1 - 1 ) );
}
``````

That looks right, doesn’t it? Maybe… If you just glance at it…

Start with 1 and bit shift it left \$1 - 1 times. So if we are validating a UINT8, we take 1 and bit shift it left 7 times which gives us… 128. Uh, that’s not right.

It should be written like this:

``````elsif ( \$constraint =~ /^UINT(\d+)/ )
{
\$start = 0;
\$end = ( 0x1 << \$1 ) - 1;
}
``````

Start with 1 and bit shift it left \$1 times and subtract 1. So given a UINT8 again, we take a 1 and bit shift it left 8 times and subtract 1 leaving us with 255. Good.

It might be better if we did away with the bit shifts altogether though and did it using a power of 2 like this instead:

``````elsif ( \$constraint =~ /^UINT(\d+)/ )
{
\$start = 0;
\$end = 2 ** \$1 - 1;
}
``````

I haven’t looked at the commit to see who made this mistake because there’s about a 50/50 chance that it was me.

:) Btw, is there something special for writing 1 as 0x1?

I like to use the hexadecimal notation just to further announce that I'm thinking about bits and storage.

However, I think I might have written this code less C-like so I could get rid of the conditional:

\$start = 0;
\$end = \$upper_bound{ \$constraint };

You might define the hash like you did for your fix, but at least you can hide that somewhere out of the way.

Aside, I stopped reading DailyWTF a while ago. At a certain point it all starts to sound like "...blah blah blah, new job, blah blah blah, no VCS/bug tracker, blah blah blah legacy code..." and the occasional code example where someone wrote a 25 level of cascading if/else chain. Eventually, this became boring.