Qual'è il modo migliore per gestire gli errori nel codice?

Il punto di scrivere eccezioni personalizzate è che hai intenzione di fare qualcosa di utile con loro.

Li sta mostrando all'utente "utile"?
Probabilmente no. Sembrano spaventosi.

Li sta registrando in un file per un esame successivo "utile"?

Forse, specialmente se l'applicazione sta per scavalcare e morire perché lo stai facendo in un gestore di eccezioni "globale" (che è circa tutto per cui vanno bene).

cattura un particolare tipo di eccezione e lo gestisce (ad esempio, scrive il codice per affrontare il problema mentre accade e corregge quel problema, preferibilmente senza che l'utente lo sappia qualsiasi cosa a riguardo) "utile"?
Oh si!

Perché usare l'eccezione personalizzata Tipi ? Perché è così che la maggior parte delle lingue si aspetta di identificare eccezioni. Guarda le clausole "catch": cercano specifici tipi di eccezioni. Mi spiace dirlo ma non sarei assolutamente d'accordo con la raccomandazione del tuo amico - lanciando la stessa [classe] ValidationException [oggetti] ovunque, ma facendo affidamento sulla proprietà Testo per spiegare cosa sta succedendo. Questo è utile solo in uno scenario - in cui mostri le eccezioni direttamente all'utente e questa è una pratica piuttosto scarsa, nel mio libro comunque.

In questa particolare situazione, un'opzione migliore potrebbe essere quella di usare il metodo Single di LINQ, in questo modo:

public void Update()
{
    var user = db.users.Single(u => u.userid == id);
    // update the user record
}

Single fa già esattamente ciò che vuoi: genera un'eccezione se ci sono 0 o più di 1 risultati corrispondenti al predicato.

A questo punto puoi decidere se sei contento dell'eccezione generata da Single per presentare una bolla, o se desideri racchiuderla in una con un messaggio più utile o un tipo specifico. Lo schema generale per questo sarebbe:

public void Update()
{
    try
    {
        var user = db.users.Single(u => u.userid == id);
    }
    catch(SomeExceptionType ex)
    {
        throw new SomeOtherExceptionType("Useful message here", ex);
    }
    // update the user record
}

(Si noti che passa ex al costruttore SomeOtherExceptionType qui. Ciò consente di conservare le informazioni sull'eccezione originale, come di solito è una buona pratica generale)

Come ho detto nel commento, la scelta di come esattamente ciò che fai non è probabilmente eccessivamente importante. Il mio consiglio è di iniziare con l'opzione più semplice, in questo caso consentire a Single di generare la propria eccezione e il refactator secondo necessità.

Se ti trovi con la necessità di visualizzare o registrare un messaggio di eccezione più specifico, avvolgi l'eccezione in questo metodo o la catena di chiamata più in alto, a seconda dei casi. Il principio su dove lo fai dovrebbe essere quello di evitare perdite tra i tuoi livelli di astrazione. Considera quanto segue:

public void HighLevelMethod()
{
    try
    {
        DataAccessClass.Update();
    }
    catch(Exception ex)
    {
        throw new SomeKindOfException("What should I say here?", ex);
    }
}

Il messaggio qui dovrebbe essere a un livello di astrazione appropriato per HighLevelMethod . Qualcosa sulla falsariga di "L'aggiornamento fallito" (anche se idealmente qualcosa di un po 'più utile!). Il motivo esatto per cui l'aggiornamento fallisce è nascosto all'interno di Update , quindi il messaggio deve essere "L'aggiornamento non è riuscito perché non è stato trovato nessun utente" causerebbe una perdita di dettagli di implementazione tra i livelli di astrazione. Se volessi specificare quel livello di dettaglio, quel messaggio dovrebbe andare in un'eccezione generata da Update stesso.

Allo stesso modo, solo refactoring per lanciare la tua sottoclasse Exception se questo fornisce alcune informazioni utili che ti trovi ad aver bisogno di un livello più alto nella catena di chiamate. Il codice che chiama Update() ha un modo diverso di gestire un UserNotFoundException rispetto a un ValidationException ? In caso contrario, non preoccuparti del tuo tipo Exception .

Se stai scrivendo una libreria che deve essere utilizzata da qualche altro codice esterno, devi essere un po 'più proattivo nell'allestire quando fornisci messaggi speciali o tipi Exception personalizzati saranno utili, piuttosto che sto solo aspettando che ci sia bisogno. Ma si applicano gli stessi principi generali.

Che cosa ha più senso per te? ValidationException descrive accuratamente il problema? Se è così, non c'è niente di sbagliato nell'usarlo così com'è e trasmettere un messaggio valido e valido insieme ad esso. Se si desidera creare un messaggio di errore più specifico, creare una classe Exception che erediti ValidationException, ad esempio "UserNotFoundException" e "ManyUsersFoundException". Questi sono probabilmente nomi poveri e non quello che vuoi, ma ti viene l'idea.

Anche ...

var user = db.users.FirstOrDefault(u => u.userid == id);

Il tuo codice qui è un po 'ridondante per usare "FirstOrDefault". Sai già, a questo punto, hai un record. Quindi puoi tranquillamente usare:

var user = db.users.First(u => u.userid == id);

E farla franca senza conseguenze.

Oltre a questo, direi che le tue modifiche basate sulla tua chat con il tuo amico mi sembrano più che sufficienti. Ma questa è un'opinione personale.

Fondamentalmente dipende da come vuoi gestire le tue dichiarazioni di cattura. Il vantaggio di avere le tue eccezioni personalizzate è che puoi prenderle separatamente ed eseguire pezzi di codice separati, vedi sotto. Tuttavia, se pianifichi di gestire tutti i tipi di eccezioni allo stesso modo di scaricare il messaggio in un file di registro o in una finestra di messaggio, utilizzare un'eccezione generica va bene.

catch (UserNotFoundException ex) 
{ 
    //handle code for user's not found
}
catch (MultipleUsersFoundException ex) 
{
    //handle code for multiple users
}
catch (Exception ex) 
{ 
    //handle other badness
}

Non utilizzo Entity Framework ma penso che si applichi la stessa regola. Quello che faccio di solito è racchiudere tutte le chiamate al database all'interno di una lambda e riutilizzare quel blocco di codice per ogni operazione. Al di fuori del lambda uso un blocco try / catch, quindi ogni eccezione viene instradata allo stesso percorso, quindi non ho bisogno di trattare ogni errore da solo. Se il risultato si verifica per generare un'eccezione, eseguo il rollback altrimenti eseguo il commit.

Se c'è un messaggio speciale che ho bisogno di inviare informazioni al client, di solito lancio un'eccezione interna nel codice lambda in modo che diventi un bubbling fuori dal lambda e il client riceve un messaggio non tecnico.

A mio parere tutte le eccezioni dovrebbero essere registrate in qualche modo, di nuovo lo faccio ad un livello più alto, al di fuori della lambda come avvertimento (per esempio un errore di integrità referenziale non è necessario un bug, ma un valore mancante).

Come per la logica di inserimento / aggiornamento che stai mostrando dipende se stai usando un metodo per la creazione e un altro metodo per l'aggiornamento. Se questo è il caso, allora si dovrebbe sollevare un'eccezione se l'id non viene trovato nel metodo di aggiornamento. Tuttavia, di solito mi piace "upsert" piuttosto che avere due metodi così come fai tu, se l'id non è presente nel database, presumo che questo sia un inserto.

Forse esaminando questo esempio sarà chiaro cosa intendo: Wrapper del database

TL; DR;

Quello che vorrei dire è che quelle non sono eccezioni a te perché dovresti averle controllate prima di chiamare l'aggiornamento. Quando arrivi a quel punto di aggiornamento, l'unica cosa che dovrebbe fallire è un errore di rete, db lock o db non disponibile; comportamento fuori dal tuo controllo. Se controlli tutto ciò di cui hai il controllo, il tuo flusso applicativo sarà tuo da controllare e non basato su eccezioni. Le eccezioni sono costose quindi dovresti evitare di usarle per controllare il flusso delle applicazioni.