2014-09-29 10 views
5

Podczas wywoływania Single lub SingleOrDefault na IEnumerable<T>, i ma więcej niż jeden wynik, rzuca InvalidOperationException.Po Single i SingleOrDefault rzucanie bardziej zwięzły wyjątek

Podczas gdy rzeczywisty komunikat wyjątku jest bardzo opisowy, problematyczne jest pisanie kodu, który będzie obsługiwał tylko przypadki, w których nie powiodą się połączenia Single/SingleOrDefault.

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

W tym scenariuszu, jeśli IsPromoCodeValid rzuca również InvalidOperationException, wówczas staje się niejednoznaczna, co jest haczyk jest obsługa.

Mogę sprawdzać wiadomość o wyjątku, ale chciałbym tego uniknąć, ponieważ uważam, że zabrudzeniem jest obsługa kodu w zależności od komunikatu o wyjątku.

Mój obecny alternatywą dla SingleOrDefault wygląda następująco:

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(); 
} 

Jednak kod ten jest dużo mniej oczywiste niż kod powyżej, dodatkowo, to generuje mniej efektywne zapytania (jeśli przy użyciu LINQ ORM) niż przy użyciu SingleOrDefault.

Mogę również zrobić Take(2) z moim drugim przykładem, aby zoptymalizować nieco, ale to dalej zaciemnia cel kodu.

Czy istnieje sposób, aby to zrobić bez pisania własnego rozszerzenia zarówno dla IEnumerable i IQueryable?

+1

Być może ma 'IsPromoCode' throw niestandardowy wyjątek? Ale ciekawe pytanie. – BradleyDotNET

+0

Czy rozważałeś utworzenie dobrze nazwanej metody rozszerzenia, która wykonuje twoją metodę 'take (2)'? – Khan

+0

'Mogę również zrobić Take (2) z moim drugim przykładem, aby zoptymalizować go nieco, ale to dalej zaciemnia cel kodu" - IMO, 'Take (2)' z zaznaczeniem 'fees.Count> 1 'czyni to bardziej zrozumiałym – Habib

Odpowiedz

2

Czy to rozwiąże 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(); 
    } 
} 
1

InvalidOperationException jest raczej ogólny. Każda z dostępnych właściwości (lub nawet głębiej w stosie) może rzucić ten wyjątek. Dlatego jednym ze sposobów jest rozwinięcie własnej metody wyjątków i rozszerzeń. Na przykład:

static class EnumerableExtensions 
{ 
    public static TSource ExactlyOneOrZero<TSource>(
     this IEnumerable<TSource> source, 
     Func<TSource, bool> predicate) 
    { 
     if (source == null) { throw new ArgumentNullException("source"); } 
     if (predicate == null) { throw new ArgumentNullException("predicate"); } 

     IEnumerable<TSource> matchingItems = source.Where(predicate); 
     IReadOnlyList<TSource> limitedMatchingItems = matchingItems.Take(2).ToList(); 

     int matchedItemCount = limitedMatchingItems.Count; 

     switch (matchedItemCount) 
     { 
      case 0: return default(TSource); 
      case 1: return limitedMatchingItems[0]; // Or Single() 
      default: throw new TooManyMatchesException(); 
     } 
    } 
} 

class TooManyMatchesException : Exception { /* Don't forget to implement this properly. */ } 

To pozwala zachować oryginalny kod czysty:

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

Innym sposobem, aby to zrobić, jest użycie TryGet... -pattern, ale to nie jest bardzo czysty. Wartość TryGetSingle zwróci wartość true, nawet jeśli nie ma pasujących elementów. Możesz zastąpić boolean przez enum (Valid/Invalid), ale pozostawiam to czytnikowi, czy jest on czytelny czy nie.

0

Uważam, że First()/Single()/SingleOrDefault() jako rodzaj Assert.

tj. Jeśli ich używasz, nie chcesz wychwycić wyjątku. Coś jest bardzo nie tak z twoimi danymi i powinno być traktowane jako błąd krytyczny.

Jeśli w twoim modelu wiele wyników jest normalnych, nie używaj wyjątków do ich weryfikacji.

Z tej perspektywy nie sądzę, aby Twoja wersja Take (2) była mniej oczywista.

Powiązane problemy