What bothers you most about this code?

Posted in Technicalon Apr 1, 2008

I came across this line of code today, and while I don’t normally hate other people’s coding style, this one has something in it that really bothers me:

i = ((unsigned int)(crcAccum >> 24) ^ *dataBlkPtr++ ) & 0xff;



April 1st, 2008 at 2:04 pm

That’s pretty gnarly. Please tell me that’s not part of a for loop declaration.

As far as what bothers me about it, it reminds me a lot of some 324 homework…

What is it checking for? I’m assuming i is going to be checked against another mask?

And I’m _really_ not a fan of casting things to unsigned ints that weren’t originally intended to be unsigned.


April 2nd, 2008 at 8:24 am

Its one of two lines in a CRC checksum calculation. Whats really scary to me is the *dataBlkPtr++, because the order of operations is not very clear to the code reader. Does it post-increment the dereferenced value, or does it post-increment the pointer after it provides the dereferenced value.

It does the latter, but the beginning programmer would have to look at least twice at it.


April 18th, 2008 at 10:37 am

The fact that the assignment has side effects bugs me. It’s too easy to introduce errors that way.

Comment Form