views:

4335

answers:

6

Hi,

I'm using linq to filter a selection of MessageItems. The method I've written accepts a bunch of parameters that might be null. If they are null, the critea for the file should be ignored. If it is not null, use it to filter the results.

It's my understanding that when doing an || operation is C#, if the first expression is true, the second expression should not be evaluated.

Eg.

if(ExpressionOne() || ExpressionTwo())
{
     // only ExpressionOne was evaluated because it was true
}

now, in linq, I'm trying this:

var messages = (from msg in dc.MessageItems
where  String.IsNullOrEmpty(fromname) || (!String.IsNullOrEmpty(fromname) && msg.FromName.ToLower().Contains(fromname.ToLower()))
select msg);

I would have thought this would be sound, because String.IsNullOrEmpty(fromname) woudl equal true and the second part of the || wouldn't get run.

However it does get run, and the second part

msg.FromName.ToLower().Contains(fromname.ToLower()))

throws a null reference expection (because fromname is null)!! - I get a classic "Object reference not set to an instance of an object" exception.

Any help?!!?

Need more info? Just ask!

Thanks in advance!

+4  A: 

Are you sure it's 'fromname' that's null and not 'msg.FromName' that's null?

Brian
Hey - thanks for the quick reply!Yeah, fromname is null.I tested by removing the ToLower() and I don't the the problemo.
Ev
So just to clarify - it is fromname that is null - not msg.FromName.Any other ideas?
Ev
What @ShuggyCoUk said. What is the type of dc.MessageItems? Does it have a fancy LINQ provider?
Brian
Hi Brian.It's my own class. I'm using Linq to SQL, so it's an object, that basically represents a table. Members are things like msg.FromName, msg.Email, msg.Location, msg.message Text - no crazy provider though.So dc.MessageItems, is a collection of MessageItembasically, I just need to know why the second part of the or operatoinis being evaluated, even though the first expression is "True"! :) Thanks for the help!
Ev
A: 

Like Brian said, I would look if the msg.FromName is null before doing the ToLower().Contains(fromname.ToLower()))

Nordes
Cheers Nordes. Like I said, I'm sure that msg.FromName is not null in this case.What do you guys think about the second part of the where getting evaluated? Am I right that it shouldn't be?
Ev
+8  A: 

Have a read of this documentation which explains how linq and c# can experience a disconnect.

Since Linq expressions are expected to be reduced to something other than plain methods you may find that this code breaks if later it is used in some non Linq to Objects context.

That said

String.IsNullOrEmpty(fromname) || 
(   !String.IsNullOrEmpty(fromname) && 
    msg.FromName.ToLower().Contains(fromname.ToLower())
)

Is badly formed since it should really be

String.IsNullOrEmpty(fromname) || 
msg.FromName.ToLower().Contains(fromname.ToLower())

which makes it nice and clear that you are relying on msg and msg.FromName to both be non null as well.

To make your life easier in c# you could add the following string extension method

public static class ExtensionMethods
{
    public static bool Contains(
        this string self, string value, StringComparison comparison)
    {
        return self.IndexOf(value, comparison) >= 0;
    }

    public static bool ContainsOrNull(
        this string self, string value, StringComparison comparison)
    {
        if (value == null)
            return false;
        return self.IndexOf(value, comparison) >= 0;
    }
}

Then use:

var messages = (from msg in dc.MessageItems
where  msg.FromName.ContainsOrNull(
    fromname, StringComparison.InvariantCultureIgnoreCase)
select msg);

However this is not the problem. The problem is that the Linq to SQL aspects of the system are trying to use the fromname value to construct the query which is sent to the server.

Since fromname is a variable the translation mechanism goes off and does what is asked of it (producing a lower case representation of fromname even if it is null, which triggers the exception).

in this case you can either do what you have already discovered: keep the query as is but make sure you can always create a non null fromname value with the desired behaviour even if it is null.

Perhaps better would be:

IEnumerable<MessageItem> results;
if (string.IsNullOrEmpty(fromname))
{ 
    results = from msg in dc.MessageItems 
    select msg;    
}
else
{
    results = from msg in dc.MessageItems 
    where msg.FromName.ToLower().Contains(fromname) 
    select msg;    
}

This is not so great it the query contained other constraints and thus invovled more duplication but for the simple query actually should result in more readable/maintainable code. This is a pain if you are relying on anonymous types though but hopefully this is not an issue for you.

ShuggyCoUk
Ev
Extension methods should not handle a null instance as that's not allowed with a normal method call in C#. "foo.Bar()" should fail with a NullReferenceException regardless of whether "Bar()" is a method or extension method.
Dan
foo.Bar shoudld fail is foo is nullbut what I'm asking is, what if I sayif(foo == null || foo.Bar())That shouldn't throw a null ref. should it? So why is it throwing this in Linq? I have a feeling there's something really funamental I'm missing here!! Do let me know if you get me! I'm not using any extendsions at all here. :)
Ev
@Dan - reread the code - I cheack *value* for null. not self
ShuggyCoUk
@Ev I'm suggesting you use the Extension method, then ordering *doesn't matter* you push the problem down into a readable and functional extension method
ShuggyCoUk
@Ev your impression that (a == null || use(a)) is legit is **wrong** in Linq plain and simple sorry. Did you try my extension method with the revisesed where clause?
ShuggyCoUk
@ShuggyCoUk: I think I get you now. Write an extension that will do this "if" for me. I'm going to try that right now. Sounds reasonable. Do you have an explanation on why it's **wrong**? Just 'cause I'm curious, because the logic seems sound to me. Perhaps it's evaluated differently. I looked through those docs you mentioned and the "if" I'm doing isn't mentioned there, but perhaps it's implied by other rules, which might have been over my head. I'll check it again.
Ev
since things like sql engines are not guaranteed to do the sort of short circuit evaluation (without resorting to crappy things like case) the Linq system makes no guarantees that this happens (hence the document about it). You don't have to write the extension - it's already there in the answer.
ShuggyCoUk
@Shuggy. Sorry about the late reply - got pulled off the proj (you know how it is) but I'm back on it now. I've just implemented your extension method which is very elegent, but still doesn't work. I get the following exception thrown: Method 'Boolean ContainsOrNull(System.String, System.String, System.StringComparison)' has no supported translation to SQL. http://social.msdn.microsoft.com/Forums/en-US/linqprojectgeneral/thread/ce3e3721-ded0-4dc6-a40a-ddeb981c2ebeWhich I'm looking at now
Ev
So I'm not at a loose end yet! I'm checking it out as we speak.I'm sure I'm on the right track. Man, the solution must be so close!Again, appreciate the time/help! Thanks!I'll be plowing away at this again today and this afternoon, so hopefully I'll come up with something and post it. Any more help would be great :)
Ev
Ah ok. I asumed you were pulling the data back then filtering after. This isn't going to work. I will edit accordingly
ShuggyCoUk
Okay. Well thanks for the help. When I find a solution I'll be sure to post it. Tanks
Ev
Whatever your solution is has to basically supply a non null value to the Contains or avoid it all together (as mine does) sorry
ShuggyCoUk
Yeah I think you're totally right.Seems odd to me, becuase it such a common thing to do. Oh wel...nice one :)
Ev
@Shuggy: i just posted this on another message, and thought I'd add it here. Just thought of a way to clarify what I'm after. I need the Linq to SQL equivalent of:SELECT * FROM tblSomeTable WHERE @theValue IS NULL OR Lower(@theValue) = Lower(theValue)
Ev
The problem here is that, on sql server Lower(@variable) is null when @variable is null. In Linq since the variable is available at the point it tries to call the ToLower (in the .Net sense of the word) first before sending the query, this crashes. This behaviour may change in future (though I think it unlikely) but I know of no way to make Linq to SQL defer evaluation of methods on variables it knows about (as opposed to column identifiers).
ShuggyCoUk
+2  A: 

Okay. I found A solution.

I changed the offending line to:

where (String.IsNullOrEmpty(fromemail)  || (msg.FromEmail.ToLower().Contains((fromemail ?? String.Empty).ToLower())))

It works, but it feels like a hack. I'm sure if the first expression is true the second should not get evaluated.

Would be great if anyone could confirm or deny this for me...

Or if anyone has a better solution, please let me know!!!

Ev
A: 

You are correct that the second conditional shouldn't get evaluated as you are using the short-circuit comparitors (see http://stackoverflow.com/questions/361069/what-is-the-best-practice-concerning-c-short-circuit-evaluation), however I'd suspect that the Linq might try to optimise your query before executing it and in doing so might change the execution order.

Wrapping the whole thing in brackets also, for me, makes for a clearer statement as the whole 'where' condition is contained within the parenthases.

Lazarus
@Lazarus. Thanks for the comment. It sounds really feasible that Linq is optimising these shortcircut comparitors and losing the functionality I'm relying on.Any ideas how to force it in Linq?Thanks for the comment!
Ev
Should have mentioned I've tried lots of different parenthases options!
Ev
+2  A: 

If you are using LINQ to SQL, you cannot expect the same C# short-circuit behavior in SQL Server. See this question about short-circuit WHERE clauses (or lack thereof) in SQL Server.

Also, as I mentioned in a comment, I don't believe you are getting this exception in LINQ to SQL because:

  1. Method String.IsNullOrEmpty(String) has no supported translation to SQL, so you can't use it in LINQ to SQL.
  2. You wouldn't be getting the NullReferenceException. This is a managed exception, it would only happen client-side, not in SQL Server.

Are you sure this is not going through LINQ to Objects somewhere? Are you calling ToList() or ToArray() on your source or referencing it as a IEnumerable<T> before running this query?


Update: After reading your comments I tested this again and realized some things. I was wrong about you not using LINQ to SQL. You were not getting the "String.IsNullOrEmpty(String) has no supported translation to SQL" exception because IsNullOrEmpty() is being called on a local variable, not an SQL column, so it is running client-side, even though you are using LINQ to SQL (not LINQ to Objects). Since it is running client-side, you can get a NullReferenceException on that method call, because it is not translated to SQL, where you cannot get a NullReferenceException.

One way to make your solution seem less hacky is be resolving fromname's "null-ness" outside the query:

string lowerfromname = String.IsNullOrEmpty(fromname) ? fromname : fromname.ToLower();

var messages = from msg in dc.MessageItems
               where String.IsNullOrEmpty(lowerfromname) || msg.Name.ToLower().Contains(lowerfromname)
               select msg.Name;

Note that this will not always be translated to something like (using your comments as example):

SELECT ... FROM ... WHERE @theValue IS NULL OR @theValue = theValue

Its translation will be decided at runtime depending on whether fromname is null or not. If it is null, it will translate without a WHERE clause. If it is not null, it will translate with a simple "WHERE @theValue = theValue", without null check in T-SQL.

So in the end, the question of whether it will short-circuit in SQL or not is irrelevant in this case because the LINQ to SQL runtime will emit different T-SQL queries if fromname is null or not. In a sense, it is short-circuited client-side before querying the database.

Lucas
I'm calling ToList() which is when the linq gets run, but not before.That's fine if I can't expect the same short circuit behaviour. I'm happy with that now. I did find a solution, but I still am not really happy with it. My solution was to changeString.IsNullOrEmpty(fromname) || msg.FromName.ToLower().Contains(fromname.ToLower())towhere ((fromname == null) || (msg.FromName.IndexOf(fromname) >= 0))so I don't assume fromname is not null. I don't think this is the best way.I want something that would turn into the equivalent of
Ev
SELECT * FROM tblSomeTableWHERE @theValue IS NULL OR @theValue = theValue
Ev
So if you have any ideas, that'd be great!
Ev
thanks, your comments made me realize i made a mistake. i have updated my answer.
Lucas