CodeSOD: Switching Notes

"The app I work on is a 1.2MLOC big-ball-o-wtf," writes Mark B.

As with a lot of big piles of bad code, it's frequently hard to find a snippet that both represents the bad code and is concise enough to submit. In this case, the code in question shows a questionable grasp of both switch statements and enums.

// Default to expire note today var noteDuration = NoteDurationType.ExpireToday; switch (note.NoteDuration) { case NoteDurationType.LengthOfStay: noteDuration = NoteDurationType.LengthOfStay; break; case NoteDurationType.ExpireToday: // Default is to expire today break; } // Save note, expiry date is set in this method and the Expiry date passed in the mobile json is ignored. Note.Note.CreateNewTaskNote(oc, note.NoteId, trimmedNote, scheduleTask.AssetTreeId, ScheduleStartDate, noteDuration)

So, a few things. First, NoteDurationType has only two possible values: ExpireToday and LengthOfStay. This code defaults the variable noteDuration to ExpireToday, then does a switch- if note.NoteDuration is LengthOfStay, set the variable noteDuration to that, otherwise, leave it alone.

So, this entire switch could be replaced by noteDuration = note.NoteDuration. The effect is the same. But then, the variable noteDuration is only used once- on the following line where we create a new task note. Which means we could replace all this code with:

Note.Note.CreateNewTaskNote(oc, note.NoteId, trimmedNote, scheduleTask.AssetTreeId, ScheduleStartDate, note.NoteDuration);

Even if we're being generous, and say that this is some misguided null check, note.NoteDuration isn't nullable, so there's no need for any of this.

It's easy to write 1.2M lines of code if most of them are stupid.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published.