views:

300

answers:

9

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?

+1  A: 

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
+2  A: 

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

Khadaji
Yes, but the OrEmpty part of that provides extra functionality, this just seems like it's adding more typing for no benefit.
Davy8
I personally like the IsNullOrEmpty as its the 'ol two birds one stone approach.
CmdrTallen
@Davy8 - Good point
Jaimal Chohan
`String.IsNullOrEmpty` is not defined as an extension method on `String`, however.
Pavel Minaev
Yes but String.IsNullOrEmpty is a static member that takes a string and not called on a null instance of a string. A rather big difference IMO.
csharptest.net
+7  A: 

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
+1  A: 

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
A: 

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
+2  A: 

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
`string.IsNullOrWhiteSpace` is a new method in .NET 4 (http://msdn.microsoft.com/en-us/library/system.string.isnullorwhitespace(VS.100).aspx).
Martinho Fernandes
+1 that's great!
Botz3000
A: 

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
+1  A: 

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
A: 

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 :-)

CodeMonkey