CodeSOD: IsEmptyOrNullOrNullOrEmpty

Peter was tracking down some bugs, when he found code which looks like this:

if (IsEmptyOrNull(myInput))
{
// do things that clearly expect myInput to NOT be null or empty
} else {
throw BadInputException(“The input must not be null.”);
}

Names are made up above, to illustrate the flow of code.

This seemed wildly wrong, and was possibly the source of the bug, so Peter dove in. Unfortunately, this wasn’t the bug. You see, IsEmptyOrNull is not a built-in function. But it wraps one.

public bool IsEmptyOrNull(string param1)
{
return !String.IsNullOrEmpty(param1);
}

Wrapping a small built-in function is already a code smell. Making the name almost identical but not quite is also a code smell. But reversing the meaning because you reversed the name is absolutely bonkers.

Did they think that A or B != B or A? Because that’s what this implies. The fact that anyone used this function, when its usage was so clearly contradicting its name, speaks to a deep level of nobody caring.

It was, at least, an easy refactoring. But it speaks to how thoroughly broken their codebase is.

Source: Read More