Ben ran into some misbehaving C# code- handling users was not doing what it was supposed to do, and when "what it's supposed to do" is "prevent users from seeing content owned by other users without permission", that's a bad thing.

The code Ben found wasn't the cause of the bug, but it ended up wasting a bunch of his time as he tried to understand why it existed.

It starts with:

public const string UserLess = null;
public const string EmptyUser = "";

Now, one might wonder why we create constants for well known values, like null and an empty string (aka String.Empty). Well, if you were reading this code, you'd have to scroll through several hundred lines before you find out where they're used.

public static bool IsUserLess([CanBeNull] string userCode)
{
  return userCode == UserLess;
}

public static bool IsSpecificUser([CanBeNull] string userCode)
{
  if (IsUserLess(userCode) || userCode == EmptyUser)
  {
    return false;
  }
  return true;
}

IsUserLess is a very verbose null check. IsSpecificUser is a very verbose IsNullOrEmpty check, which just so happens to be a built-in method. These functions could be replaced with !string.IsNullOrEmpty(userCode). I don't even hate creating the IsSpecificUser function as a wrapper, just to give it a domain-specific name, but the additional constants make the whole thing confusing.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!