Having Single and SingleOrDefault to throw a more succinct exception

Question

When invoking Single or SingleOrDefault on an IEnumerable<T>, and it has more than one result, it throws InvalidOperationException.

While the actual message of the exception is very descriptive, it is problematic to write a catch that will only handle the cases where the Single / SingleOrDefault calls fail.

public virtual Fee GetFeeByPromoCode(string promoCode)
{
    try
    {
        return _fees.SingleOrDefault(f => f.IsPromoCodeValid(promoCode));
    }
    catch (InvalidOperationException)
    {
        throw new TooManyFeesException();
    }
}

In this scenario, if IsPromoCodeValid also throws an InvalidOperationException, then it becomes ambiguous as to what the catch is handling.

I could inspect the message of the exception, but I would like to avoid that as I find it dirty to handle code depending on a message of an exception.

My current alternative to the SingleOrDefault looks like the following:

public virtual Fee GetFeeByPromoCode(string promoCode)
{
    var fees = _fees.Where(f => f.IsPromoCodeValid(promoCode)).ToList();

    if (fees.Count > 1)
    {
        throw new InvalidFeeSetupException();
    }

    return fees.FirstOrDefault();
}

However, this code is a lot less obvious than the code above, in addition, this generates a less efficient query (if using a linq-enabled ORM) than using SingleOrDefault.

I could also do a Take(2) with my second example to optimize it a bit, but this further obfuscates the intent of the code.

Is there a way to do this without writing my own extension for both IEnumerable and IQueryable?

Answer 1

May this solve the problem?

public virtual Fee GetFeeByPromoCode(string promoCode)
{
    try
    {
        return _fees.SingleOrDefault(f =>
            {
                try
                {
                    return f.IsPromoCodeValid(promoCode);
                }
                catch(InvalidOperationException)
                {
                    throw new PromoCodeException();
                }
            });
    }
    catch (InvalidOperationException)
    {
        throw new TooManyFeesException();
    }
}

Answer 2

I consider First() / Single() / SingleOrDefault() as a kind of Assert.

i.e. If you use them you don't want to catch the exception. Something is very wrong with your data and it should be handled as a critical error.

If multiple results is normal in your model, don't use exceptions to verify it.

From that perspective I don't think your Take(2) version is less obvious.