Is extending String class with IsNullOrEmpty confusing?

Question

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;
  }
}

Answer 1

I really like this approach AS LONG AS the method makes it clear that it is checking the object is null. ThrowIfNull, IsNull, IsNullOrEmpty, etc. It is very readable.

Answer 2

I think the root of this problem is what Jon Skeet has mentioned in the list of things he hates in his favorite language (C#): C# should not have imported all extension methods in a whole namespace automatically. This process should have been done more explicitly.

My personal opinion about this specific question is (since we can't do anything about the above fact) to use the extension method if you want. I don't say it won't be confusing, but this fact about extension methods (that can be called on null references) is a global thing and doesn't affect only String.IsNullOrEmpty, so C# devs should get familiar with it.

By the way, it's fortunate that Visual Studio clearly identifies extension methods by (extension) in the IntelliSense tooltip.

Answer 3

I think extending any object with an "IsNull" type call is a little confusing and should be avoided if possible.

It's arguable that the IsEmptyString method might be useful for the String type, and that because you'd usually combine this with a test for null that the IsNullOrEmpty might be useful, but I'd avoid this too due to the fact that the string type already has a static method that does this and I'm not sure you're saving yourself that much typing (5 characters at most).

Answer 4

Personally, I wouldn't create an extension that does something that already exists in the framework unless it was a significant improvement in usability. In this instance, I don't think that's the case. Also, if I were to create an extension, I would name it in a way as to reduce confusion, not increase it. Again, I think this case fails that test.

Having said all that, I do have a string extension that tests, not only if the string is null or empty, but also if it only contains whitespace. I call it IsNothing. You can find it here.

Answer 5

It doesn't just look like you're calling a method on a null variable. You /are/ calling a method on a null variable, albeit one implemented through a static extension method. I had no idea extension methods (which I was already leery of) supported that. This even allows you to do crazy things like:

public static int PowerLength(this string obj)
{
return obj == null ? 0 : obj.Length;
}

From where I'm standing now, I would classify any use of an extension method on a null reference under considered harmful.

Answer 6

Calling a method on a variable that is null usually results in a NullReferenceException. The IsNullOrEmpty()-Method deviates from this behaviour in a way that is not predictable from just looking at the code. Therefore I would advise against using it since it creates confusion and the benefit of saving a couple of characters is minimal.

Answer 7

Yes, it will confuse.

I think that everyone who knows about IsNullOrEmpty() perceives the use of it as quite natural. So your suggested extension method will not add more readability.

Maybe for someone new to .NET this extension method might be easier to deal with, but there is the danger possibility that she/he doesn't understand all facets of extension methods (like that you need to import the namespace and that it can be invoked on null). She/he will might wonder why this extension method does not exist in another project. Anyway: Even if someone is new to .NET the syntax of the IsNullOrEmpty() method might become natural quite fast. Also here the benefits of the extension methods will not outweight the confusion caused by it.

Edit: Tried to rephrase what I wanted to say.

Answer 8

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.

Answer 9

In general, I'm only ok with extension methods being safe to call on null if they have the word 'null' or something like that in their name. That way, I'm clued in to the fact that they may be safe to call with null. Also, they better document that fact in their XML comment header so I get that info when I mouse-over the call.

Answer 10

Let's look at the pro:s and con:s...

1. More readable.

Yes, slightly, but the improvement in readability is outweighed by the fact that it looks like you are calling an instance method on something that doesn't have to be an instance. In effect it's easier to read, but it's harder to understand, so the improved readability is really just an illusion.

1. Less typing.

Yes, but that is really not a strong argument. If typing is the main part of your programming, you are just not doing something that is remotely challenging enough for you to evolve as a developer.

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

True, (as mentioned above).

Answer 11

If I'm not mistaken, every answer here decries the fact that the extension method can be called on a null instance, and because of this they do not support believe this is a good idea.

Let me counter their arguments.

I don't believe AT ALL that calling a method on an object that may be null is confusing. The fact is that we only check for nulls in certain locations, and not 100% of the time. That means there is a percentage of time where every method call we make is potentially on a null object. This is understood and acceptable. If it wasn't, we'd be checking null before every single method call.

So, how is it confusing that a particular method call may be happening on a null object? Look at the following code:

var bar = foo.DoSomethingResultingInBar();
Console.Writeline(bar.ToStringOr("[null]"));

Are you confused? You should be. Because here's the implementation of foo:

public Bar DoSomethingResultingInBar()
{
  return null; //LOL SUCKER!
}

See? You read the code sample without being confused at all. You understood that, potentially, foo would return a null from that method call and the ToStringOr call on bar would result in a NRE. Did your head spin? Of course not. Its understood that this can happen. Now, that ToStringOr method is not familiar. What do you do in these situations? You either read the docs on the method or examine the code of the call. Here it is:

public static class BarExtensions
{
  public static string ToStringOr(this bar, string whenNull)
  {
    return bar == null ? whenNull ?? "[null]" : bar.ToString();
  }
}

Confusing? Of course not. Its obvious that the developer wanted a shorthand method of checking if bar is null and substituting a non-null string for it. Doing this can slash your code significantly and increase readability and code reuse. Of course you could do this in other ways, but this way would be no more confusing than any other. For example:

var bar = foo.DoSomethingResultingInBar();
Console.Writeline(ToStringOr(bar, "[null]"));

When you encounter this code, what do you have to differently than the original version? You still have to examine the code, you still have to determine its behavior when bar is null. You still have to deal with this possibility.

Are extension methods confusing? Only if you don't understand them. And, quite frankly, the same can be said for ANY part of the language, from delegates to lambdas.