Parametro per controllare se lanciare un'eccezione o restituire null - buona pratica?

23

Spesso mi imbatto in metodi / funzioni che hanno un ulteriore parametro booleano che controlla se un'eccezione viene lanciata in caso di errore o se viene restituito un valore nullo.

Ci sono già discussioni su quale di queste sia la scelta migliore, nel qual caso, quindi non concentriamoci su questo qui. Vedi per es. Restituisci valore magico, genera un'eccezione o restituisci false su fallimento?

Supponiamo invece che ci sia una buona ragione per cui vogliamo supportare entrambi i modi.

Personalmente ritengo che un metodo del genere dovrebbe essere diviso in due: uno che genera un'eccezione in caso di errore, l'altro che restituisce null in caso di errore.

Quindi, che è meglio?

A: Un metodo con parametro $exception_on_failure .

/**
 * @param int $id
 * @param bool $exception_on_failure
 *
 * @return Item|null
 *   The item, or null if not found and $exception_on_failure is false.
 * @throws NoSuchItemException
 *   Thrown if item not found, and $exception_on_failure is true.
 */
function loadItem(int $id, bool $exception_on_failure): ?Item;

B: due metodi distinti.

/**
 * @param int $id
 *
 * @return Item|null
 *   The item, or null if not found.
 */
function loadItemOrNull(int $id): ?Item;

/**
 * @param int $id
 *
 * @return Item
 *   The item, if found (exception otherwise).
 *
 * @throws NoSuchItemException
 *   Thrown if item not found.
 */
function loadItem(int $id): Item;

EDIT: C: Qualcos'altro?

Molte persone hanno suggerito altre opzioni, o affermano che sia A sia B sono difettosi. Tali suggerimenti o opinioni sono benvenuti, pertinenti e utili. Una risposta completa può contenere tali informazioni aggiuntive, ma affronterà anche la questione principale se una modifica o un comportamento di un parametro sia una buona idea.

Note

Nel caso qualcuno si stia chiedendo: gli esempi sono in PHP. Ma penso che la domanda si applichi attraverso le lingue purché siano in qualche modo simili a PHP o Java.

    
posta donquixote 23.12.2017 - 12:45
fonte

10 risposte

35

Hai ragione: due metodi sono molto migliori per questo, per diversi motivi:

  1. In Java, la firma del metodo che potenzialmente genera un'eccezione includerà questa eccezione; l'altro metodo non lo farà. Rende particolarmente chiaro cosa aspettarsi dall'uno e dall'altra.

  2. In lingue come C # in cui la firma del metodo non dice nulla sulle eccezioni, i metodi pubblici dovrebbero essere ancora documentati e tale documentazione include le eccezioni. La documentazione di un singolo metodo non sarebbe facile.

    Il tuo esempio è perfetto: i commenti nel secondo pezzo di codice sembrano molto più chiari, e vorrei anche abbreviare "L'oggetto, se trovato (eccezionalmente diverso)" fino a "L'oggetto". - La presenza di un potenziale eccezione e la descrizione che ci hai fornito è autoesplicativa.

  3. In un caso di un singolo metodo, ci sono pochi casi in cui desideri commutare il valore del parametro booleano su runtime , e se lo fai, significherebbe che il il chiamante dovrà gestire entrambi i casi (una null risposta e l'eccezione), rendendo il codice molto più difficile di quanto non debba essere. Dato che la scelta è fatta non in fase di runtime, ma quando si scrive il codice, due metodi hanno perfettamente senso.

  4. Alcuni framework, come .NET Framework, hanno convenzioni stabilite per questa situazione e lo risolvono con due metodi, proprio come suggerivi tu. L'unica differenza è che usano uno schema spiegato da Ewan in la sua risposta , quindi int.Parse(string): int genera un'eccezione, mentre int.TryParse(string, out int): bool no. Questa convenzione di denominazione è molto strong nella comunità di .NET e dovrebbe essere seguita ogni volta che il codice corrisponde alla situazione che descrivi.

risposta data 23.12.2017 - 13:16
fonte
9

Una variante interessante è la lingua Swift. In Swift, dichiarate una funzione come "lancio", cioè è consentito lanciare errori (simili ma non proprio come dire eccezioni Java). Il chiamante può chiamare questa funzione in quattro modi:

a. In una dichiarazione try / catch che cattura e gestisce l'eccezione.

b. Se il chiamante viene dichiarato come lancio, basta chiamarlo e ogni errore generato diventa un errore lanciato dal chiamante.

c. Contrassegnare la chiamata di funzione con try! che significa "Sono sicuro che questa chiamata non verrà lanciata". Se la funzione chiamata fa gira, l'applicazione è garantita in crash.

d. Contrassegnare la chiamata di funzione con try? che significa "So che la funzione può essere lanciata ma non mi interessa quale sia l'errore che viene lanciato". Questo cambia il valore di ritorno della funzione dal tipo " T " al tipo " optional<T> " e un'eccezione generata viene trasformata in return nil.

(a) e (d) sono i casi che stai chiedendo. Cosa succede se la funzione può restituire nil e può generare eccezioni? In quel caso il tipo del valore restituito sarebbe " optional<R> " per un certo tipo R, e (d) lo cambierebbe in " optional<optional<R>> " che è un po 'criptico e difficile da capire ma completamente soddisfacente. E una funzione Swift può restituire optional<Void> , quindi (d) può essere usato per le funzioni di lancio che restituiscono Void.

    
risposta data 23.12.2017 - 21:48
fonte
2

Mi piace l'approccio bool TryMethodName(out returnValue) .

Fornisce un'indicazione conclusiva sul fatto che il metodo abbia avuto o meno esito positivo e che il nome corrisponda al wrapping del metodo di lancio delle eccezioni in un blocco catch try.

Se restituisci null, non sai se ha fallito o se il valore di ritorno è stato legittimamente nullo.

es:

//throws exceptions when error occurs
Item LoadItem(int id);

//returns false when error occurs
bool TryLoadItem(int id, out Item item)

esempio di utilizzo (out significa passaggio per riferimento non inizializzato)

Item i;
if(TryLoadItem(3, i))
{
    Print(i.Name);
}
else
{
    Print("unable to load item :(");
}
    
risposta data 23.12.2017 - 12:52
fonte
2

Normalmente un parametro booleano indica che una funzione può essere divisa in due e, di norma, devi dividerlo sempre piuttosto che passare in un booleano.

    
risposta data 23.12.2017 - 23:32
fonte
1

Pulisci codice consiglia di evitare gli argomenti del flag booleano, perché il loro significato è opaco nel sito di chiamata:

loadItem(id, true) // does this throw or not? We need to check the docs ...

mentre metodi separati possono usare nomi espressivi:

loadItemOrNull(id); // meaning is obvious; no need to refer to docs
    
risposta data 24.12.2017 - 15:19
fonte
1

Se dovessi usare A o B, userei B, due metodi separati, per i motivi indicati in Arseni Risposta di Mourzenkos .

Ma c'è un altro metodo 1 : in Java è chiamato Optional , ma puoi applicare lo stesso concetto ad altre lingue dove puoi definire i tuoi tipi, se la lingua non è già fornire una classe simile.

Definisci il tuo metodo in questo modo:

Optional<Item> loadItem(int id);

Nel tuo metodo restituisci Optional.of(item) se trovi l'articolo o restituisci Optional.empty() nel caso in cui non lo fai. Non passi mai 2 e non restituisci mai null .

Per il client del tuo metodo è qualcosa come null , ma con la grande differenza che costringe a pensare al caso degli articoli mancanti. L'utente non può semplicemente ignorare il fatto che non ci può essere alcun risultato. Per ottenere l'elemento da Optional è richiesta un'azione esplicita:

  • loadItem(1).get() genererà un NoSuchElementException o restituirà l'elemento trovato.
  • C'è anche loadItem(1).orElseThrow(() -> new MyException("No item 1")) per usare un'eccezione personalizzata se il% di ritorno restituito è vuoto.
  • Optional restituisce l'elemento trovato o il% co_de passato (che potrebbe anche essere loadItem(1).orElse(defaultItem) ) senza generare un'eccezione.
  • defaultItem restituirebbe ancora una null con il nome degli articoli, se presente.
  • Ci sono altri metodi, vedi Javadoc per loadItem(1).map(Item::getName) .

Il vantaggio è che ora è fino al codice cliente che cosa dovrebbe accadere se non ci sono articoli e comunque devi solo fornire un singolo metodo .

1 Credo che sia qualcosa come Optional nella risposta di gnasher729s ma non è del tutto chiaro per me visto che non conosco Swift e sembra essere una funzionalità linguistica, quindi è specifico di Swift.

2 Puoi generare eccezioni per altri motivi, ad es. se non sai se è presente o meno un articolo perché hai avuto problemi di comunicazione con il database.

    
risposta data 30.12.2017 - 09:12
fonte
0

Come altro punto concordante con l'utilizzo di due metodi, questo può essere molto facile da implementare. Scriverò il mio esempio in C #, poiché non conosco la sintassi di PHP.

public Widget GetWidgetOrNull(int id) {
    // All the lines to get your item, returning null if not found
}

public Widget GetWidget(int id) {
    var widget = GetWidgetOrNull(id);

    if (widget == null) {
        throw new WidgetNotFoundException();
    }

    return widget;
}
    
risposta data 23.12.2017 - 21:27
fonte
0

Preferisco due metodi, in questo modo non solo mi dirai che stai restituendo un valore, ma quale valore esattamente.

public int GetValue(); // If you can't get the value, you'll throw me an exception
public int GetValueOrDefault(); // If you can't get the value, you'll return me 0, since it's the default for ints

Anche TryDoSomething () non è un cattivo modello.

public bool TryParse(string value, out int parsedValue); // If you return me false, I won't bother checking parsedValue.

Sono più abituato a vedere il primo nelle interazioni del database mentre il secondo è più utilizzato nelle analisi.

Come altri hanno già detto, i parametri flag di solito sono un odore di codice (gli odori di codice non significano che stai facendo qualcosa di sbagliato, solo che c'è una probabilità che tu stia sbagliando). Sebbene tu lo chiami con precisione, a volte ci sono sfumature che rendono difficile sapere come usare quella bandiera e finisci per guardare all'interno del corpo del metodo.

    
risposta data 27.12.2017 - 17:49
fonte
-1

Mentre altre persone suggeriscono il contrario, suggerirei di avere un metodo con un parametro per indicare come devono essere gestiti gli errori è meglio che richiedere l'uso di metodi separati per i due scenari di utilizzo. Anche se può essere desiderabile che anche abbia punti di ingresso distinti per gli "errori di lancio" rispetto a "errori restituisce l'indicazione di errore", l'uso di un punto di ingresso che può servire entrambi i modelli di utilizzo eviterà la duplicazione del codice nei metodi wrapper . Come un semplice esempio, se uno ha funzioni:

Thing getThing(string x);
Thing tryGetThing (string x);

e si vogliono le funzioni che si comportano allo stesso modo ma con x racchiuso tra parentesi, sarebbe necessario scrivere due set di funzioni wrapper:

Thing getThingWithBrackets(string x)
{
  getThing("["+x+"]");
}
Thing tryGetThingWithBrackets (string x);
{
  return tryGetThing("["+x+"]");
}

Se c'è un argomento su come gestire gli errori, sarebbe necessario un solo wrapper:

Thing getThingWithBrackets(string x, bool returnNullIfFailure)
{
  return getThing("["+x+"]", returnNullIfFailure);
}

Se il wrapper è abbastanza semplice, la duplicazione del codice potrebbe non essere un problema, ma se dovesse mai dover cambiare, la duplicazione del codice richiederà a sua volta la duplicazione di eventuali modifiche. Anche se si opta per aggiungere punti di ingresso che non usano l'argomento extra:

Thing getThingWithBrackets(string x)
{
   return getThingWithBrackets(x, false);
}
Thing tryGetThingWithBrackets(string x)
{
   return tryGetThingWithBrackets(x, true);
}

si può mantenere tutta la "logica aziendale" in un unico metodo, quello che accetta un argomento che indica cosa fare in caso di errore.

    
risposta data 23.12.2017 - 21:36
fonte
-1

Penso che tutte le risposte che spiegano che non si debba avere un flag che controlli se un'eccezione è lanciata o meno sono buone. Il loro consiglio sarebbe il mio consiglio a chiunque senta il bisogno di fare quella domanda.

Tuttavia, volevo sottolineare che esiste un'eccezione significativa a questa regola. Una volta che sei abbastanza avanzato per iniziare a sviluppare API che verranno utilizzate da centinaia di altre persone, ci sono casi in cui potresti voler fornire una tale bandiera. Quando stai scrivendo un'API, non stai solo scrivendo ai puristi. Stai scrivendo a clienti reali che hanno desideri reali. Parte del tuo lavoro è renderli felici con la tua API. Questo diventa un problema sociale, piuttosto che un problema di programmazione, ea volte i problemi sociali dettano soluzioni che sarebbero non ideali come soluzioni di programmazione per conto proprio.

Potresti scoprire di avere due utenti distinti della tua API, uno dei quali desidera eccezioni e uno che non lo fa. Un esempio di vita reale in cui ciò potrebbe verificarsi è con una libreria che viene utilizzata sia in fase di sviluppo che in un sistema embedded. Negli ambienti di sviluppo, gli utenti probabilmente vorranno avere eccezioni ovunque. Le eccezioni sono molto popolari per la gestione di situazioni impreviste. Tuttavia, sono proibiti in molte situazioni incorporate perché sono troppo difficili da analizzare intorno ai vincoli in tempo reale. Quando ci si preoccupa non solo del tempo medio necessario per eseguire la propria funzione, ma anche del tempo necessario per ogni singolo divertimento, l'idea di srotolare lo stack in qualsiasi luogo arbitrario è molto indesiderabile.

Un vero esempio di vita per me: una biblioteca matematica. Se la tua libreria matematica ha una classe Vector che supporta il recupero di un vettore unitario nella stessa direzione, devi essere in grado di dividere per la grandezza. Se la magnitudine è 0, si ha una situazione di divisione per zero. Nello sviluppo vuoi catturare questi . Non vuoi davvero sorprendere il divario per 0 che gironzola. Lanciare un'eccezione è una soluzione molto popolare per questo. Non succede quasi mai (è un comportamento veramente eccezionale), ma quando lo fa, vuoi saperlo.

Sulla piattaforma integrata, non vuoi quelle eccezioni. Avrebbe più senso fare un controllo if (this->mag() == 0) return Vector(0, 0, 0); . In effetti, lo vedo in codice reale.

Ora pensa dalla prospettiva di un'azienda. Puoi provare a insegnare due diversi modi di utilizzare un'API:

// Development version - exceptions        // Embeded version - return 0s
Vector right = forward.cross(up);          Vector right = forward.cross(up);
Vector localUp = right.cross(forward);     Vector localUp = right.cross(forward);
Vector forwardHat = forward.unit();        Vector forwardHat = forward.tryUnit();
Vector rightHat = right.unit();            Vector rightHat = right.tryUnit();
Vector localUpHat = localUp.unit()         Vector localUpHat = localUp.tryUnit();

Questo soddisfa le opinioni della maggior parte delle risposte qui, ma dal punto di vista aziendale questo non è auspicabile. Gli sviluppatori incorporati dovranno imparare uno stile della codifica e gli sviluppatori di sviluppo dovranno imparare un altro. Questo può essere piuttosto fastidioso e costringe le persone a pensare in un modo. Potenzialmente peggiore: come si fa a dimostrare che la versione integrata non include alcun codice di lancio delle eccezioni? La versione di lancio può facilmente confondersi, nascondendosi da qualche parte nel codice fino a quando una costosa Failure Review Board lo trova.

Se, d'altra parte, hai una bandiera che attiva o disattiva la gestione delle eccezioni, entrambi i gruppi possono apprendere lo stesso identico stile di codifica. In molti casi, infatti, puoi persino utilizzare il codice di un gruppo nei progetti dell'altro gruppo. Nel caso di questo codice che usa unit() , se ti interessa davvero cosa succede in un caso di divisione per 0, dovresti aver scritto tu stesso il test. Quindi, se lo sposti su un sistema embedded, dove ottieni solo risultati cattivi, quel comportamento è "sano di mente". Ora, dal punto di vista del business, alleno una volta i miei programmatori e utilizzano le stesse API e gli stessi stili in tutte le parti della mia attività.

Nella grande maggioranza dei casi, si desidera seguire il consiglio degli altri: usare idiomi come tryGetUnit() o simili per le funzioni che non generano eccezioni e restituiscono invece un valore sentinella come null. Se ritieni che gli utenti possano voler utilizzare sia la gestione delle eccezioni sia il codice di gestione senza eccezioni affiancati all'interno di un singolo programma, segui la notazione tryGetUnit() . Tuttavia, esiste un caso limite in cui la realtà aziendale può rendere migliore l'uso di una bandiera. Se ti trovi in quel caso d'angolo, non aver paura di gettare le "regole" sul ciglio della strada. Ecco a cosa servono le regole!

    
risposta data 24.12.2017 - 17:38
fonte

Leggi altre domande sui tag