2014-09-29 7 views
5

Quando si richiama Single o SingleOrDefault su un IEnumerable<T>, e ha più di un risultato, si getta InvalidOperationException.Avendo unico e SingleOrDefault ad un'eccezione più succinta

Mentre il messaggio reale del eccezione è molto descrittivo, è problematico scrivere un fermo che gestirà solo i casi in cui i Single/SingleOrDefault chiamate non riescono.

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

In questo scenario, se IsPromoCodeValid getta anche una InvalidOperationException, allora diventa ambigua su ciò che il fermo sta gestendo.

ho potuto ispezionare il messaggio di eccezione, ma vorrei evitare che, come trovo sporca per gestire il codice a seconda di un messaggio di un'eccezione.

Il mio attuale alternativa al SingleOrDefault si presenta come la seguente:

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

Tuttavia, questo codice è molto meno evidente rispetto al codice di cui sopra, in aggiunta, questo genera una query meno efficiente (se si utilizza un LINQ abilitato ORM) rispetto all'utilizzo di SingleOrDefault.

Potrei anche fare un Take(2) con il mio secondo esempio per ottimizzarlo un po ', ma questo ulteriormente offusca l'intento del codice.

C'è un modo per farlo senza scrivere la mia estensione per entrambi IEnumerable e IQueryable?

+1

Forse 'IsPromoCode' genera un'eccezione personalizzata? Domanda interessante però. – BradleyDotNET

+0

Hai pensato di creare un metodo di estensione ben chiamato che usa il metodo 'take (2)'? – Khan

+0

'Potrei anche fare un Take (2) con il mio secondo esempio per ottimizzarlo un po ', ma questo in più offusca l'intento del codice. - IMO,' Take (2) 'con il controllo' fees.Count> 1 'lo rende più chiaro – Habib

risposta

2

Può questo risolvere il problema?

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 è piuttosto generale. Qualsiasi proprietà a cui si accede (o anche più in profondità nello stack) potrebbe lanciare questa eccezione. Pertanto, un modo è quello di eseguire il proprio metodo di eccezione e estensione. Per esempio:

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. */ } 

Ciò consente di mantenere il codice originale pulito:

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

Un altro modo per fare questo, è quello di utilizzare il TryGet... -pattern, ma non è molto pulito. TryGetSingle restituisce true anche se non ci sono elementi corrispondenti. È possibile sostituire il booleano con un enum (valido/non valido), ma lascerò al lettore se questo è leggibile o meno.

0

Considero First()/Single()/SingleOrDefault() come una sorta di Assert.

Ad esempio, se li si utilizza, non si desidera rilevare l'eccezione. Qualcosa è molto sbagliato con i tuoi dati e dovrebbe essere gestito come un errore critico.

Se nel modello sono presenti più risultati, non utilizzare eccezioni per verificarlo.

Da quella prospettiva non penso che la versione Take (2) sia meno ovvia.

Problemi correlati