News & Articles:

WotW: Code Smell

Doing a code review today, we found something that was not quite a bug. Then we learned that it had a name. It became our new Word of the Week.

The code itself looked fine, was quite simple, but when you looked closer was a little odd. This is what it did:

  • It created a temporary local variable.
  • Inside a loop, it called a bunch of methods which returned integer values.
  • It kept a running total of the return values in the local variable.
  • When it exited the loop, it did absolutely nothing with the value in the local variable.

Now, what the code needed to do was call the methods. The return values weren’t really useful in this case (for good reasons it matttered not whether methods succeeded or failed), so ignoring them was a reasonable plan. But: Why maintain that running total?

The code wasn’t just ignoring the return values (which would have been fine), it was doing something with them. And then ignoring the result. That had been done deliberately, and although the code worked, it looked odd.

We duly raised this as an issue, and then learned that this sort of thing has a name.

Yes, it was a Code Smell. We liked the name and thought it was worth sharing.

Posted in Technical, Views

Tags: , ,