SingleOrDefault exception handling

Question

I have a samle code which calls SingleOrDefault method 3 times and logs exception if any sequence has more than one matching element.

The problem starts if I want to check which part of this code throws exception.

Is it possible to get some useful information from this exception like predicate parameter or collection type for more detailed trace?

like this - Sequence contains more than one matching element. Collection IEnumrable|ParamType| param {Predicate param toString()}

 public void GetSingleOrDefaultTest(){

    try{

        var user = Users.SingleOrDefault(e => e.Id == 1);

        var profile = UserProfiles.SingleOrDefault(e => e.Id == 1);

        var profile2 = UserProfiles.SingleOrDefault(e => e.Id == 2);


    } catch(InvalidOperationException ex){
        Log(ex);
    }

}

See: http://stackoverflow.com/questions/1745691/linq-when-to-use-singleordefault-vs-firstordefault-with-filtering-criteria — Meysam Tolouee, May 15 '14 at 09:56
You're doing it wrong in the first place. Don't rely on exception handling for control flow. You'll accidentally catch true bugs and hide them, among other problems. — usr, May 15 '14 at 10:32
@usr It's not clear to me from this snippet alone that it is being used for control flow — Ben Aaronson, May 15 '14 at 10:43
@BenAaronson if exceptions are an expected part of execution (and avoidable), then they are used for control flow. That would be my definition. — usr, May 15 '14 at 10:48
@usr Yeah I'm just not sure that's the case in this situation. Why would multiple users with the same ID be an expected part of execution? This looks like standard "check my data didn't somehow get into a weird unexpected state" code — Ben Aaronson, May 15 '14 at 11:14
Yes it's not a best-practice code but If I need something in case of supporting not very properly written code with a lot of such statements or having not appropriate data it's useful to have ability to find problematic place without rewriting it. — Alex, May 15 '14 at 11:27

abto · Accepted Answer · 2017-03-03T06:56:45.177

If you want to know which of the statement issues the error you have to check them separateley. Catch the InvalidOperationException on every SingleOrDefault invocation and wrap it in a new exception which you can fill with additional information.

try
{
    User user;
    UserProfile profile;
    UserProfile profile2;

    try
    {
        user = Users.SingleOrDefault(e => e.Id == 1);
    }
    catch (InvalidOperationException ex)
    {
        throw new InvalidOperationException("User lookup for Id = 1 failed", ex);
    }

    try
    {
        profile = UserProfiles.SingleOrDefault(e => e.Id == 1);
    }
    catch (InvalidOperationException ex)
    {
        throw new InvalidOperationException("User profile lookup for Id = 1 failed", ex);
    }

    try
    {
        profile2 = UserProfiles.SingleOrDefault(e => e.Id == 2);
    }
    catch (InvalidOperationException ex)
    {
        throw new InvalidOperationException("User profile lookup for Id = 2 failed", ex);
    }

    // work with user, profile and profile2
}
catch(InvalidOperationException ex)
{
    Log(ex);
}

Edit:

You also can encapsulate the single try catches by the following

private static T GetSingleOrDefault<T>(IEnumerable<T> collection, Expression<Func<T, bool>> predicate)
{
    try
    {
        return collection.SingleOrDefault(predicate.Compile());
    }
    catch (InvalidOperationException e)
    {
        var message = string.Format(
            "{0} (Collection: {1}, param: {2})",
            e.Message,
            collection.GetType(),
            predicate);

        throw new InvalidOperationException(message);
    }
}

so that your code would look like

try
{
    var user = GetSingleOrDefault(Users, e => e.Id == 1);

    var profile = GetSingleOrDefault(UserProfiles, e => e.Id == 1);

    var profile2 = GetSingleOrDefault(UserProfiles, e => e.Id == 2);

    // work with user, profile and profile2
}
catch(InvalidOperationException ex)
{
    Log(ex);
}

This yields in a message like

System.InvalidOperationException: Sequence contains more than one matching element (Collection: IEnumerable`1[User], param: e => e.Id == 1)

score 1 · Answer 2 · edited May 23 '17 at 12:34

1

Whenever you use SingleOrDefault, you clearly state that the query should result in at most a single result. On the other hand, when FirstOrDefault is used, the query can return any amount of results but you state that you only want the first one.

I personally find the semantics very different and using the appropriate one, depending on the expected results, improves readability.

Reference

edited May 23 '17 at 12:34

Community

1
1

answered May 15 '14 at 09:58

Meysam Tolouee

569
3
17

This is true, but in this case I think `SingleOrDefault` is what he actually wants – Ben Aaronson May 15 '14 at 11:45

score 1 · Answer 3 · answered May 15 '14 at 10:01

There is no reasone to use SingelOrDefault. I would refactor this to:

var user = Users.Count(e => e.Id == 1);
var profile = UserProfiles.Count(e => e.Id == 1);
var profile2 = UserProfiles.Count(e => e.Id == 2);

if(user + profile + profile2 != 3){
  Log("more than one");
}

This do basically the same but is not Exception driven. And I don't see in your question a reasone use exception driven programing.

score 0 · Answer 4 · answered May 26 '14 at 09:08

That's where logging comes into scene (NLog usage sample, available from NuGet):

public void GetSingleOrDefaultTest()
{
    try
    {
        Logger.Debug("Getting user with id = {0}", 1);
        var user = Users.SingleOrDefault(e => e.Id == 1);

        Logger.Debug("Getting user profile with id = {0}", 1);
        var profile = UserProfiles.SingleOrDefault(e => e.Id == 1);

        Logger.Debug("Getting user profile with id = {0}", 2);
        var profile2 = UserProfiles.SingleOrDefault(e => e.Id == 2);
    } 
    catch(Exception ex)
    {
        Logger.ErrorException("Failed getting single or default", ex);
    }
}

One glance at log file will tell you which statement has failed:

2014-05-26 12:04:48.8655 DEBUG Getting user with id = 1
2014-05-26 12:04:48.8815 DEBUG Getting user profile with id = 1
2014-05-26 12:04:48.8815 DEBUG Getting user profile with id = 2
2014-05-26 12:04:48.8815 ERROR Failed getting single or default
Sequence contains more than one matching element

score -2 · Answer 5 · answered May 15 '14 at 11:00

Before I answer: Please be aware of the perils of using exception handling here. In the comments in this question I have pointed out a few problems. In the end you might worsen your log output by logging misleading information.

Don't rely on exception handling for control flow. You'll accidentally catch true bugs and hide them, among other problems.

Exceptions also have very bad performance.

It is very error prone to use exception handling to diagnose specific issues. Let's see if we can get away without using it.

public static TSource SingleOrDefaultWithDiagnostics<TSource>(
  this IEnumerable<TSource> source,
  Func<TSource, bool> predicate,
  string failureMessage) {

    using (IEnumerator<TSource> enumerator = source.Where(predicate).GetEnumerator())
    {
        if (!enumerator.MoveNext())
        {
            return default(TSource);
        }
        TSource current = enumerator.Current;
        if (!enumerator.MoveNext())
        {
            return current;
        }
        else throw new SingleElementException(failureMessage);
    }
}

I've taken some BCL code and changed the way the error is thrown.

This outputs a meaningful exception in exactly the case that you are looking for. You can attach a custom diagnostic message.

 var user = Users.SingleOrDefaultWithDiagnostics(e => e.Id == 1, "User with Id 1");
 var profile = UserProfiles.SingleOrDefaultWithDiagnostics(e => e.Id == 1, "...");
 var profile2 = UserProfiles.SingleOrDefaultWithDiagnostics(e => e.Id == 2, "...");

Could you explain the point of this method? It looks like you've just reimplemented `SingleOrDefault` — Ben Aaronson, May 15 '14 at 11:28
@BenAaronson you can now tell from the exception exactly what happened. That is not the case with the original which throws an undifferentiated IOE. — usr, May 15 '14 at 11:29
The documentation for `SingleOrDefault` says it throws an IOE if "source has more than one element." That's the same as yours, except you're using a different exception type. — Ben Aaronson, May 15 '14 at 11:32
The predicate could throw as well, or there might be some surrounding code that also can throw. — usr, May 15 '14 at 11:41
But in this case, neither of those are true (yes okay, `Id` could have some weird `get` logic, but it almost certainly doesn't). This is very confusing advice to give as an answer to this question. — Ben Aaronson, May 15 '14 at 11:50

SingleOrDefault exception handling

5 Answers5

Linked