current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite
1

Here is code for check null in object:

public static TResult IfNotNull<TInput, TResult>(this TInput obj, Func<TInput,TResult> expression)
    {
        if (obj == null || expression == null) return default(TResult);
        var value = expression(obj);
        return value;
    }

The above code is best pratice of check nullability in object. I'm working on c# V4.0.

Example usage :

Person.IfNotNull(x => x.User).IfNotNull(x => x.Name) ?? "";
share|improve this question
up vote 4 down vote accepted

To be sure it always works you should add a constraint to the method:

where TInput : class

This will ensure that TInput is a reference type.

It makes no sense to call it on structs. For them, you need another overload with a different constraint and some ? question marks:

public static TResult IfNotNull<TInput, TResult>(
     this TInput? obj, 
     Func<TInput, TResult> expression
) where TInput : struct
{
    if (!obj.HasValue) return default(TResult);
    var value = expression(obj.Value);
    return value;
}

Someone may ask at this point: but why do we need two extensions if theoretically a single one does the job too?

Consider this:

((decimal?)2).IsNotNull(x => x.Value * 2)

With a single extension you need to use the .Value property for nullable types. With two extensions you get a clean value so you just do 

((decimal?)2).IsNotNull(x => x * 2)

But isn't this just a convenience? Of course it is. Don't we write extensions exactly for that reason?

As a matter of fact I use a similar code myself:

public static TResult IIf<TArg, TResult>
(
    this TArg arg,
    Func<TArg, bool> predicate,
    Func<TArg, TResult> ifTrue,
    Func<TArg, TResult> ifFalse = null
)
{
    if (predicate == null) { throw new ArgumentNullException(nameof(predicate)); }
    if (ifTrue == null) { throw new ArgumentNullException(nameof(ifTrue)); }

    return predicate(arg) ? ifTrue(arg) : (ifFalse == null ? default(TResult) : ifFalse(arg));
}

but you can create a simplified version of your method and combine it with the IIf

public static bool IsNotNull<TInput>(this TInput obj) where TInput : class
{
    return obj != null;
}

to do this

Person
    .IIf(p => p.IsNotNull(), p => p.User)
    .IIf(u => u.IsNotNull(), u => u.Name, u string.Empty);
share|improve this answer
    
There's no need for the class constraint. The method will work just fine for nullable structs when just using the OP's method. There's also no real reason to prohibit the method from being called on a non-nullable value. Yes, it's pointless, but it's still entirely functional. There's nothing gained from your alteration. – Servy Nov 8 '16 at 20:53
    
@Servy ok, noted but you should know that there is a reason why the constraint makes sense, especially for nullable types; consider this:((decimal?)2).IsNotNull(x => x.Value) here you need to explicitly use the .Value property because it passed it entirely. With an additional extension you get a clean value without x.Value. – t3chb0t Nov 8 '16 at 21:08
    
Okay, you can potentially omit some number of calls to Value. The point remains that you specifically said that your transformation is necessary, when in fact it's only a very mild possible convenience. Big difference. – Servy Nov 8 '16 at 21:12
    
@Servy I understand your point. I should have mentioned it. Somehow I thought it was obvious since I used the expressionobj.Value there. Of course it's a convenience, all extensions are about convenience. I'll add this additional example another time. – t3chb0t Nov 8 '16 at 21:16
up vote 2 down vote

If you're using C# 6 it would be much cleaner to just use the null-conditional operator, ?.:

Person?.User?.Name

This will return null if either Person, User or Name is null.

If you're not using C# 6 you should strongly consider upgrading to the current version of Visual Studio (2015). You can continue to target .Net 3.5 while still benefitting from the many new features and improvements developed over the past 5+ years.

share|improve this answer
1  
The OP writes: I'm working on c# V4.0. – t3chb0t Nov 8 '16 at 12:54
up vote 1 down vote

Your way seems just fine.

One thing to note is that I would throw an exception if a null "expression" parameter was provided - it is a better practice to do so, since it is not "acceptable" that the expression parameter would be null. There's a standard .NET exception for this (System.ArgumentNullException).

Also, consider renaming "expression" to "selector" - it is a common name for projection delegates in .NET

share|improve this answer
up vote 1 down vote

Not much to talk about the code as it's pretty short and readable rather your name is bad for such function it's not descriptive at all. It returns default() if the some of the parameters is null else it returns the expected result , most of the standard methods that have the same logic usually end like this OrDefault e.g FirstOrDefault. You can maybe call it ExpressionOrDefault.

You might also want to add some constraints to the generic parameter maybe let where TInput : class as value type variables can't be null anyway.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.