Null or empty object when LINQ to Entities query returns nothing

  • Say I have a LINQ query like this:

    application = CreditDatabase
                            .Applications
                            .Select(Mapper.Map<Application>) 
                            .Where(c => c.uID == urID)
                            .DefaultIfEmpty().First();
    

    It returns null if the LINQ query returns an empty result set. Is this "correct". Is it better to return an empty object? If so then how can I do that? This link says it is not possible to return a non-null reference type: https://msdn.microsoft.com/en-us/library/bb360179(v=vs.110).aspx

    @JaCquesB, What happens if there is nothing returned?

    Sorry, I was wrong, `.DefaultIfEmpty().First()` will yield `null` if the list is empty, but using `.FirstOrDefault()` would be simpler. What do you mean by "empty object", do you mean an empty list?

    Once you let nulls in you're forever needing to test for null. So have you faithfully prevented nulls from coming in else where?

    @CandiedOrange. I have to check the database to find out if an application exists.

    An empty container can sometimes be more gracefully dealt with. Depends on the container and how you use it.

    Personally I don't think 'empty' objects make a whole lot of sense with regard to databases because the existence of an object in a database is a binary condition - i.e. either it exists or it doesn't exist. That condition is neatly represented in a null-vs-not-null check. Furthermore, the 'Safe Navigation' operator in C# 6.0 can tidy some of the if-not-null clutter.

    @CandiedOrange, hence the question. How do I return an empty object? Thanks.

    null is just one way. There is also empty string, empty list, maybe monad, and my personal favorite the null object pattern.

  • Database queries return result sets. An empty set is a reasonable answer; it means you don't have any of the things searched for.

    I would argue that your code is going to far in canonicalization of the result.

    Your code (somewhere shortly after this line) is going to check to see if the application you just searched for exists or not, so you are not preventing the subsequent conditional logic (not shown in your question) in your code by doing .FirstOrDefault().

    You can — and I argue should — instead: check for the empty set (i.e. use the size of the result set, maybe with .Any()) to see if the application exists. That is substantially cleaner than converting the empty set to default and then taking the first and finally later checking that for null to see if the application of interest exists.

    Rather than going this far:

    application = CreditDatabase
                        .Applications
                        .Select(Mapper.Map<Application>) 
                        .Where(c => c.uID == urID)
                        .DefaultIfEmpty().First();
    if ( application == null ) {
       // handle missing application, maybe add it...
    } else {
        // use the found application
    }
    

    Omit the default handling; instead:

    search = CreditDatabase
                        .Applications
                        .Select(Mapper.Map<Application>) 
                        .Where(c => c.uID == urID);
    if ( ! search.Any () ) {
        // handle missing application, maybe add it...
    } else {
        application = search.Single ();
        // use the found application
    }
    

    Addendum: In the above application = search.Single (), I have used .Single() instead of .First() since excepting if the non-empty result set has more than one match makes sense here: it is a lookup by what we presume is an exact match of unique primary key (otherwise as @JacquesB points out, since the query isn't ordered, we don't really know what we're keeping and what we're throwing away with .First()).

    This is a nice solution. Do you know if that's going to issue two separate queries to the database though? (I'm betting it will.) Hitting the database twice may be undesirable.

    @RubberDuck: That can be solved by a simple `.ToList()` after the where, before the any.

  • It depends!

    But first a clarification: .DefaultIfEmpty().First() can be written simpler as .FirstOrDefault() which does the same - returns the first item, or null if the result is empty. But you probably have a bug here: First() indicates you might have multiple items and want to select the first item - but since there is no ordering involved it means you will get a random item. This is probably not what you want! If you really expect there could be multiple matches, then just return the list of matches. If you expect a single item, use Single() instead, and if you expect either a single item or nothing, use SingleOrDefult() which will also return null if there was no match.

    Now to the interesting question: What to return from a method which finds either a single object or nothing?

    There are different options depending on what you want to achieve:

    • Return null if the item is not found. Advantages: this is really simple. Disadvantages: The client have to check for null which is easily forgotten becaus the type system does not enforce this check. This might lead to hard-to-debug NullReferenceExceptions down the line. Not recommended.

    • Throw an exception if the object is not found. Advantages: Simple and safe. Disadvantages: Should only be used if it indicates a bug if the object does not exist.

    • Create a separate method which allow you to check if the object exists (eg. if (ApplicationExists(id))...). This method returns a boolean. Advantages: Clear code on the client side. Disadvantages: Will hit the database twice unless you use some kind of caching, so it may be expensive. Also more code.

    • Use the TryXXX pattern. Returns a boolean which indicates if the object exists and places the object (if present) in an out parameter. Advantages: This pattern is already known from say Dictionary.TryGetValue() so readers will not be confused. Disadvantage: The out parameter leads to somewhat ugly code, although this is improved in C# 6.

    • Return an Option<T> type. This requires the client to explicitly check and unwrap the object. Advantages: Much safer and more elegant than using null. Disadvantage: C# does not have a builtin Option<T> type, you have to find one or write one yourself.

    The only thing you should not do:

    • return an "empty" instance of the class.

    An "empty" instance does make sense in many cases e.g. loading a draft state or many situations where things are aggregatable, however, a name like `application` strongly suggests this isn't the case for this example. Boolean blindness argues against approaches via booleans. As you state, I would throw an exception if not getting a result should "never happen", otherwise I would use the `Option` approach. Since nulls are, unfortunately, already ubiquitous in C#, I may use `null` in a temporary, localized way.

  • I like retuning null when something is not found. To me it makes sense.

    Give me the first item matching this criteria.. there are none, here is a null. Your code will throw an exception if it tries to use it.

    An alternative if you don't like testing for nulls, or empty objects, though is to always return a list of objects.

    That way you can do a for each on the list and it naturally does nothing if nothing is found.

    There is no way to override the default behaviour, but you can use null concatenation

    var x = Applications.FirstOrDefault() ?? new Application();
    

    Ps. move your mapping to after the where clause!

    Thanks. Could you clarify what you mean by: "move your mapping to after the select!"

    edited to make clearer. it looks like you run the mapper against all the objects and then just select one. better to select the one first and only map the one thing

    although it may be optimised underneath

    Could you clarify what you mean by: "move your mapping to after the select!". Some code would help. Thanks.

License under CC-BY-SA with attribution


Content dated before 6/26/2020 9:53 AM