13

I like readability.

So, I came up with an extension mothod a few minutes ago for the (x =! null) type syntax, called IsNotNull. Inversly, I also created a IsNull extension method, thus

if(x == null) becomes if(x.IsNull())

and

if(x != null) becomes if(x.IsNotNull())

However, I'm worried I might be abusing extension methods. Do you think that this is bad use of Extenion methods?

Jaimal Chohan
  • 8,530
  • 6
  • 43
  • 64
  • It's a lot like http://stackoverflow.com/questions/790810/is-extending-string-class-with-isnullorempty-confusing – Vadim Sep 29 '09 at 21:50
  • 8
    It's not any shorter, and it's not any clearer. – Pavel Minaev Sep 29 '09 at 21:58
  • 1
    I actually agree with Jamial (the OP) that it's more readable. The way I see it, just about any verbal replacement for symbols and signs is an improvement in readability. But I do agree with most of the answers that it doesn't have any big impact. – MasterMastic May 05 '13 at 13:31

9 Answers9

14

It doesn't seem any more readable and could confuse people reading the code, wondering if there's any logic they're unaware of in those methods.

I have used a PerformIfNotNull(Func method) (as well as an overload that takes an action) which I can pass a quick lambda expression to replace the whole if block, but if you're not doing anything other than checking for null it seems like it's not providing anything useful.

Davy8
  • 30,868
  • 25
  • 115
  • 173
  • I do not downvote, but I complete disagree with the statement "could confuse people reading the code", I agree with CodeMokey solution and @csharptest.net – AFetter Feb 18 '20 at 22:08
4

I don't find that incredibly useful, but this:

someString.IsNullOrBlank()    // Tests if it is empty after Trimming, too
someString.SafeTrim()         // Avoiding Exception if someString is null

because those methods actually save you from having to do multiple checks. but replacing a single check with a method call seems useless to me.

Botz3000
  • 39,020
  • 8
  • 103
  • 127
3

It is perfectly valid to do but I don't think it is incredibly useful. Since extension methods are simply compiler trickery I struggle to call any use of them "abuse" since they are just fluff anyhow. I only complain about extension methods when they hurt readability.

Andrew Hare
  • 344,730
  • 71
  • 640
  • 635
2

Instead I'd go with something like:

static class Check {
    public static T NotNull(T instance) {
        ... assert logic
        return instance;
    }
}

Then use it like this:

Check.NotNull(x).SomeMethod();
y = Check.NotNull(x);

Personally it's much clearer what is going on than to be clever and allow the following:

if( ((Object)null).IsNull() ) ...
csharptest.net
  • 62,602
  • 11
  • 71
  • 89
  • I think this is ambiguous, how should a user remember that Check.NotNull(x) will return an instance of the same type! To me when I read Check.NotNull(x) in a code I expect to get true/false result. – Fred Jand Dec 02 '13 at 18:08
2

I don't entirely agree with the reasoning saying "it may confuse".

To some extent I can see what is meant, that there is no reason to venture outside "common understanding" -- everybody understands object != null.

But in Visual Studio, we have wonderful tools where you can simply hover over the method, to reveal some additional information.

If we were to say that the extension-method was annotated with a good explanation, then I feel that the argument of confusion falls apart.

The methods .IsNotNull() and .IsNull() explain exactly what they are. I feel they are very reasonable and useful.

In all honesty it is a matter of "what you like". If you feel the methods will make it more readable in the context of your project, then go for it. If you are breaking convention in your project, then I would say the opposite.

I have had the same thoughts as you have on the subject and have asked several very very experienced developers at my place of work. And none of them have come up with a good reason (except what has been mentioned about -confusion- here) that would explain why you shouldn't do this.

Go for it :-)

1

There is precedent, in as much as the string class has IsNullOrEmpty

Khadaji
  • 2,147
  • 2
  • 17
  • 19
1

You're also introducing method call overhead for something that's a CLR intrinsic operation. The JIT might inline it away, but it might not. It's a micro-perf nitpick, to be sure, but I'd agree that it's not particularly useful. I do things like this when there's a significant readability improvement, or if I want some other behavior like "throw an ArgumentNullException and pass the arg name" that's dumb to do inline over and over again.

nitzmahone
  • 13,720
  • 2
  • 36
  • 39
0

It can make sense if you, for instance, assume that you might want to throw an exception whenever x is null (just do it in the extension method). However, I my personal preference in this particular case is to check explicitly (a null object should be null :-) ).

Ludvig A. Norin
  • 5,115
  • 7
  • 30
  • 34
0

To follow the pattern it should be a property rather than a method (but of course that doesn't work with extensions).

Data values in the System.Data namespace has an IsNull property that determines if the value contains a DbNull value.

The DataRow class has an IsNull method, but it doesn't determine if the DataRow is null, it determines if one of the fields in the data row contains a DbNull value.

Guffa
  • 687,336
  • 108
  • 737
  • 1,005