Stella needs to interface with a cloud-hosted marketing automation system. The documentation isn’t particularly great, and her organization doesn’t have anyone with any serious experience with the stack, so she’s been trying to find examples and wrapper libraries that can make it easier.
She found one. While digging through the wrapper code, she found this block:
public void instantiate(string email, string password, string user_key)
{
// make sure parameters are filld out
if ( email.notFilled() || password.notFilled() || user_key.notFilled() )
{
throw new System.ArgumentException("Email, password, and user key must be provided");
}
…
}
There’s nothing horrifying there, though notFilled
isn’t a member of string
, which means it has to be an extension method- C#’s version of mixing in new functionality to existing types. This didn’t strike her as a good use of an extension method, though, since IsNullOrEmpty
is a built in string method, which would do exactly this.
bool notFilled(string str)
{
if ( str != null && str.Length > 0 )
{
return true;
}
return false;
}
Well, they did just go ahead and re-implement IsNullOrEmpty
. But they took it a step farther: notFilled
returns true
if the string is not null or empty, and false if it is. Which is the exact opposite of what’s implied by notFilled
, and is also the exact opposite of how it’s being invoked.
Not only did they reinvent built-in functionality, they named in wrong, forgot that they had, and thus called it based on what it sounds like it should do, but not what it actually does.
This code, as written, would fail with an ArgumentException
if you did supply an email, password, or user key.`