Search code examples
c#stringextension-methods

Is extending String class with IsNullOrEmpty confusing?


Everyone knows and love String.IsNullOrEmpty(yourString) method.

I was wondering if it's going to confuse developers or make code better if we extend String class to have method like this:

yourString.IsNullOrEmpty();

Pro:

  1. More readable.
  2. Less typing.

Cons:

  1. Can be confusing because yourString variable can be null and it looks like you're executing method on a null variable.

What do you think?

The same question we can ask about myObject.IsNull() method.

That how I would write it:

public static class StringExt
{
  public static bool IsNullOrEmpty(this string text)
  {
    return string.IsNullOrEmpty(text);
  }

  public static bool IsNull(this object obj)
  {
    return obj == null;
  }
}

Solution

  • I'm personally not a fan of doing this. The biggest problem with extension methods right now is discoverability. Unleses you flat out know all of the methods which exist on a particular type, it's not possible to look at a method call and know that it's an extension method. As such I find it problematic to do anything with an extension method that would not be possible with a normal method call. Otherwise you will end up confusing developers.

    A corollary to this problem exist in C++. In several C++ implementations it's possible to call instance methods on NULL pointers as long as you don't touch any fields or virtual methods on the type. I've worked with several pieces of code that do this intentionally and give methods differentt behavior when "this==NULL". It's quite maddening to work with.

    This is not to say that I don't like extension methods. Quite the contrary, I enjoy them and use them frequently. But I think there are 2 important rules you should follow when writing them.

    • Treat the actual method implementation as if it's just another static method because it in fact is. For example throw ArgumentException instead of NullReference exception for a null this
    • Don't let an extension method perform tricks that a normal instance method couldn't do

    EDIT Responding to Mehrdad's comment

    The problem with taking advantage of it is that I don't see str.IsNullOrEmpty as having a significant functional advantage over String.IsNullOrEmpty(str). The only advantage I see is that one requires less typing than the other. The same could be said about extension methods in general. But in this case you're additionally altering the way people think about program flow.

    If shorter typing is what people really want wouldn't IsNullOrEmpty(str) be a much better option? It's both unambiguous and is the shortest of all. True C# has no support for such a feature today. But imagine if in C# I could say

    using SomeNamespace.SomeStaticClass;
    

    The result of doing this is that all methods on SomeStaticClass were now in the global namespace and available for binding. This seems to be what people want, but they're attaching it to an extension method which I'm not a huge fan of.