"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.