3

I want to be able to call properties on objects that might be null but not explicitly have to check whether they are null or not when calling.

Like this:

var something = someObjectThatMightBeNull.Property;

My idea is to create a method that takes an Expression, something like this:

var something = GetValueSafe(() => someObjectThatMightBeNull.Property);

TResult? GetValueSafe<TResult>(Expression<Func<TResult>> expression) 
    where TResult : struct
{
    // what must I do?
}

What I need to do is to inspect the expression and determine if someObjectThatMightBeNull is null or not. How would I do this?

If there is any smarter way of being lazy I'd appreciate that too.

Thanks!

Mikael Östberg
  • 16,982
  • 6
  • 61
  • 79

2 Answers2

3

What you're talking about is called null-safe dereferencing - this SO specifically asks that question: C# if-null-then-null expression.

Expressions aren't really the answer (see below for clarification of my reasons for that statement). This extension method might be, though:

public static TResult? GetValueSafe<TInstance, TResult>(this TInstance instance,
  Func<TInstance, TResult> accessor)
  where TInstance : class
  where TResult : struct
{  
   return instance != null ? (TResult?)accessor(instance) : (TResult?)null;
}

And now you can do:

MyObject o = null;
int? i = o.GetValueSafe(obj => obj.SomeIntProperty);

Assert.IsNull(i);    

Obviously this is most useful when the property is a struct; you can reduce to any type and just use default(TResult) - but then you'd get 0 for ints, doubles etc:

public static TResult GetValueSafe<TInstance, TResult>(this TInstance instance,
  Func<TInstance, TResult> accessor, TResult def = default(TResult))
  where TInstance : class
{  
   return instance != null ? accessor(instance) : def;
}

This second version is more useful specifically because it works for any TResult. I've extended with an optional parameter to allow the caller to provide the default, e.g (using o from previous code):

int i = o.GetValueSafe(obj => obj.SomeIntProperty); //yields 0
i = o.GetValueSafe(obj => obj.SomeIntProperty, -1); //yields -1

//while this yields string.Empty instead of null
string s = o.GetValueSafe(obj => obj.SomeStringProperty, string.Empty);

Edit - in response to David's comment

David suggested my answer is wrong because it doesn't provide an expression-based solution, and that is what was asked for. My point is that any truly correct, and indeed responsible, answer on SO should always try to seek a simpler solution for the person asking the question if one exists. I believe it is widely accepted that over-complicated solutions to otherwise simple problems should be avoided in our day-to-day professional lives; and SO is only as popular as it is because it's community behaves in the same way.

David also took issue with my unjustified statement that 'they're not the solution' - so I'm going to expand upon that now and show why the expression-based solution is largely pointless, except in a rare edge-case that the OP doesn't actually ask for (which, incidentally, David's answer doesn't cover either).

The irony being that it makes this answer in itself perhaps unnecessarily complicated :) You can safely ignore from here down if you don't actually care why expressions aren't the best route

Whilst it is correct to say that you can solve this with expressions, for the examples laid out in the question there is simply no reason to use them - it's over-complicating what is ultimately quite a simple issue; and at runtime the overhead of compiling the expression (and subsequently throwing it away, unless you put caching in, which is going to be tricky to get right unless you emit something like call sites, like the DLR uses) is huge compared to the solution I present here.

Ultimately the motivation of any solution is to try and keep the work required by the caller to a minimum, but at the same time you also need to keep the work that is to be done by the expression analyzer to a minimum as well, otherwise the solution becomes almost unsolvable without a lot of work. To illustrate my point - let's look at the simplest we can achieve with a static method that takes an expression, given our object o:

var i = GetValueSafe(obj => obj.SomeIntProperty);

Uh-oh, that expression doesn't actually do anything - because it's not passing the o to it - the expression itself is useless to us, because we need the actual reference to o that could be null. So - the first of the solutions to this, naturally, is to explicitly pass the reference:

var i = GetValueSafe(o, obj => obj.SomeIntProperty);

(Note - could also be written as an extension method)

Thus the static method's job is to take the first parameter and pass it to the compiled expression when it invokes it. This also helps with identifying the type of the expression whose property is sought. It also, however, completely nullifies the reason to use an expression in the first place; since the method itself can immediately make the decision to access the property or not - since it has a reference to the object that could be null. Thus, in this case, it's easier, simpler and faster to simply pass a reference and accessor delegate (instead of an expression), as my extension method does.

As I mentioned, there is a way to get around having to pass the instance and that is to do one of the following:

var i = GetValueSafe(obj => o.SomeIntProperty);

Or

var i = GetValueSafe(() => o.SomeIntProperty);

We're discounting the extension method version - because with that we get a reference passed to the method, and as soon as we get a reference we can do away with expressions, as my last point proved.

Here we're relying on the caller to understand that they must include an expression that represents the actual instance (be it an in-scope property or field or local variable) in the body of the expression, on the left hand side of the member read, so that we can actually get a concrete value from it in order to do the null-check.

This is not a natural use of expression parameters, first of all, so I believe your callers could be confused. There's also another issue, which I think will be a killer if you intend to use this a lot - you cannot cache these expressions, because each time the instance, whose 'null-ness' you want to sidestep, is being baked into the expression that is passed. This means that you are always having to recompile the expression for every call; and that is going to be really slow. If you parameterise the instance in the expression you can then cache it - but then you end up with our first solution which required the instance to be passed; and again I've already shown there that we can then just use a delegate!

It is relatively easy - using the ExpressionVisitor class - to write something that can turn all property/field reads (and method calls for that matter) into 'safe' calls like you want. However, I cannot see any benefit to doing this unless you intend to do a safe read on something like this: a.b.c.d. But then the augmentation of value types to nullable versions of themselves is going to cause you a good few headaches in the expression-tree rewriting I can tell you; leaving a solution that hardly anyone will understand :)

Community
  • 1
  • 1
Andras Zoltan
  • 41,961
  • 13
  • 104
  • 160
  • That is pretty nice! It splits the expression though, meaning that it splits the object from the property. But I think it'll do. Thanks! – Mikael Östberg Apr 10 '12 at 12:36
  • It's somewhat important not to return defaults for structs. I'd rather return null for them, but that's easily done. – Mikael Östberg Apr 10 '12 at 12:38
  • Just curious, why are "expressions not really the answer" here? Was there any real reason why? – David Morton Apr 10 '12 at 12:40
  • @DavidMorton, there's no *benefit* to be gained from sticking with expressions, except perhaps for chained expressions like `a.b.c` (which will be very difficult to implement correctly anyway, e.g. what rules to use for short-circuiting if any of the left-hand expressions are null?); the expression will perform the same null check on the instance that my non-dynamic code will do but will be significantly slower - so there's no point apart from the sake of 'being clever'. – Andras Zoltan Apr 10 '12 at 12:45
  • @MikaelÖstberg I understand the struct issue - it's a little clumsier for the caller but you can use `o.GetValueSafe(obj => (int?)obj.SomeIntProperty)` if using the second version of the method. – Andras Zoltan Apr 10 '12 at 12:48
  • @DavidMorton was it you that downvoted also? If not - then 'down voter' - I fail to see what exactly is wrong with this answer. An answer that encourages the questioner *away* from an approach which is more complicated whilst achieving the same result is a good thing, surely? – Andras Zoltan Apr 10 '12 at 14:57
  • 1
    Yes, that was me. I downvoted it not based on it's encouragement to do something that might or might not be better, but based on the fact that it doesn't directly answer the question, but offers a workaround instead. Whether or not there is a "good reason" to do what the OP requested or not, on a StackOverflow page, is less important than answering the OP's question. The "which is better to do, this or that", is really a different question entirely. People who come to this page, will likely be looking for the answer, not a workaround. – David Morton Apr 10 '12 at 15:26
  • @DavidMorton I ***strongly*** disagree with that sentiment - if you look around you will find literally thousands questions where the highested voted and accepted answers discouraged the user from their initial thoughts; that's what sharing experience and knowledge is all about. It's happened to me on my own questions and I have been very grateful for that, as the questioner here has. – Andras Zoltan Apr 10 '12 at 15:35
  • We'll have to agree to disagree on that. That being said, you've encouraged me to revise my original post. – David Morton Apr 10 '12 at 15:44
  • Nice that I got to affect you both this much and nice that you agreed to disagree. While I thought expressions was the way I wanted to do this and I pretty much wanted expressions to be the solution, the expression based solution is not something I can leave in the source for the next developer to find. They will not get it. I eventually went with the lambda based extension method since that is easier to understand. Thank you both for your efforts in my question. Anyway, the correct answer flag goes to David though since it answers the question directly. Andras, you'll get an upvote. Cheers! – Mikael Östberg Apr 11 '12 at 09:20
3

It's complicated, but it can be done, without leaving "expression-land":

// Get the initial property expression from the left 
// side of the initial lambda. (someObjectThatMightBeNull.Property)
var propertyCall = (MemberExpression)expression.Body;

// Next, remove the property, by calling the Expression 
// property from the MemberExpression (someObjectThatMightBeNull)
var initialObjectExpression = propertyCall.Expression;

// Next, create a null constant expression, which will 
// be used to compare against the initialObjectExpression (null)
var nullExpression = Expression.Constant(null, initialObjectExpression.Type);

// Next, create an expression comparing the two: 
// (someObjectThatMightBeNull == null)
var equalityCheck = Expression.Equal(initialObjectExpression, nullExpression);

// Next, create a lambda expression, so the equalityCheck 
// can actually be called ( () => someObjectThatMightBeNull == null )
var newLambda = Expression.Lambda<Func<bool>>(equalityCheck, null);

// Compile the expression. 
var function = newLambda.Compile();

// Run the compiled delegate. 
var isNull = function();

That being said, as Andras Zoltan has so eloquently put in the comments: "Just because you can doesn't mean you should." Make sure you have a good reason to do this. If there's a better way to, then do that instead. Andras has a great workaround.

David Morton
  • 16,338
  • 3
  • 63
  • 73