Va bene lasciare che gli argomenti non validi scorrano su un altro metodo?

A parte motivo di allocazione della memoria (che, a proposito, mi sembra un'ottimizzazione prematura), ci sono altri elementi a favore di intercettare gli argomenti non validi il prima possibile:

Traccia stack

Quando si verifica un'eccezione, ti aspetti che le ultime poche righe mostrino la posizione di un errore. La ricerca dal basso verso l'alto della pila per trovare il colpevole farà perdere tempo in seguito, durante il debug, quando in particolare non avremo tempo.

Essere sciatti riguardo alla convalida dell'input e lasciare che i metodi richiamati vengano gestiti ancora e ancora significherà che la traccia dello stack sarà più grande di quanto dovrebbe essere . Se sai che ReadAll non può accettare una capacità negativa, perché aggiungere una linea per tracciare lo stack, invece di lanciare un'eccezione adesso per rendere il debug facile?

leggibilità

Il tuo codice dovrebbe essere auto-documentato . Posso solo sapere che il tuo metodo non accetta valori negativi se osservo il metodo da vicino. Questo è OK se sto per modificare il metodo; questo non è OK se voglio semplicemente utilizzare il metodo, e non mi interessa davvero come è implementato.

E no, non posso fare affidamento sulla documentazione, dato che è sempre incompleto e obsoleto.

Convalidare gli input proprio nella parte superiore del metodo aiuterebbe la leggibilità: sarebbe chiaro che non posso usare valori negativi, e vorrei evitare di sprecare altro tempo:

• Ispezionando il corpo del metodo,
• Ispezionando tutti i metodi nel codice base questo metodo chiama,
• Lettura di MSDN per ogni metodo .NET Framework chiamato da questo metodo.

Contratti di codice

Dato che hai taggato la tua domanda .net e il tuo codice sembra come C #, non posso evitare di parlare di contratti di codice. Se hai usato i contratti di codice, saresti costretto ad aggiungere comunque una Contract.Requires , altrimenti il controllore statico si lamenterà che nulla garantisce che il costruttore di liste sarà chiamato con argomenti validi.

Questo è anche il vantaggio dei contratti di codice: ti costringono a gestire gli input non validi nella parte superiore dello stack , quindi quando l'input non è valido, ti fermi proprio ora, invece di chiamare decine di altri metodi prima di incontrare un'eccezione.

La risposta è: dipende. Nel tuo esempio, dovresti lasciare che List elabori la sua eccezione e lasci che ogni eccezione bolla la catena.

Il tuo metodo di esempio non dovrebbe sapere o preoccuparsi se il valore non è valido, poiché il suo unico scopo è di passarlo a qualcos'altro. Se stavi usando il valore in ReadAll come qualcosa di diverso da un argomento su List, dovresti testarlo prima dell'uso, ma non lo sei.

Un argomento non valido per il tuo metodo non è uno che non sarà compreso da un'utilità separata, è uno che non ha senso per il tuo codice.

Come nota a margine, i parametri che esistono semplicemente per essere passati, dovrebbero essere abbastanza rari.

Risposta breve: no, non è OK. Perché anche chiamare il costruttore di List significa, dal punto di vista del compilatore: allocazione di memoria per il nuovo oggetto, scrivere riferimenti per esso nella tabella di riferimento, accedere ad altre subroutine solo per creare il nuovo oggetto List, solo per farlo esplodere nel tuo faccia generando un'eccezione.

So che non sembra molto con tutte queste risorse disponibili sul sistema, ma non è ancora qualcosa che farei - se posso evitarlo con solo una riga di codice in più.

Devi essere in disaccordo con la risposta accettata . L'argomento sull'allocazione della memoria, ecc. È rilevante solo se questo metodo è chiamato spesso con un argomento illegale.

Lo scopo di controllare gli argomenti (programmazione per contratto) e lanciare eccezioni è principalmente quello di correggere i bug durante lo sviluppo. Durante lo sviluppo, l'efficienza non è un problema. In combinazione con i test unitari, questo dovrebbe assicurare che ReadAll() sia non chiamato con un argomento negativo. O almeno chiamato solo raramente in un caso che non hai testato.

Quindi, con uno sviluppo e test adeguati, l'efficienza non sarà mai un problema. Lasciando il controllo scivolare uno o due livelli va bene. Anche se potresti voler aggiungere un commento.

EDIT # 1 aggiunto

Facciamo l'esempio leggermente diverso. La funzione restituisce il risultato sotto forma di array:

public string[] ReadAll(int maxCapacity) {...}

Vuoi controllare maxCapacity > = 0 nel tuo codice? Improbabile. IMO, questo caso di usare una lista è abbastanza vicino a una matrice che controllando la capacità iniziale è eccessiva.

EDIT # 2 Aggiunto

Ci sono molte volte in cui è del tutto irragionevole controllare da soli un argomento in arrivo. Ad esempio, supponiamo che entri in gioco una stringa XML. Il tuo metodo ha un aspetto simile a questo: (codice pseudo-Java-like, né sto affermando che questo è un buon progetto)

public double getFooBarFraction(String xml) throws AllKindsOfExceptions {
   Document doc = DOMParser.getInstance().parse(xml);
   return Double.parseDouble(doc.getElement("Foo")
                                .getElement("Bar")
                                .getAttribute("fraction"));
}

1. Convalidi tu stesso l'XML, o lascia che l'utilità DOMParser farlo?
2. Stai andando a cercare elementi e attributi te stesso, o lascia che Documento lo faccia?
3. Hai intenzione di testare questo conteggio può essere analizzato ad un doppio o lasciare che Double lo faccia?

La risposta è quasi certamente "no" a tutte delle domande. Ora, probabilmente dovresti racchiudere le chiamate nei blocchi try / catch per una migliore gestione degli errori, ma permetti comunque all'utilità di creare l'eccezione originale.

Nella rilettura delle altre risposte, dicono "dove è facile testare" ecc. Immagino che se puoi fare il test in una o due righe di codice, pensa di farlo.