Nota: una discussione più dettagliata sul codice è disponibile sul mio sito web. Copre quasi tutti i punti trattati qui e mostra, passo dopo passo, come questo pezzo di codice può essere rielaborato in un software affidabile.
Per prima cosa, puoi provare i casi limite.
-
Cosa succede se plainText è nullo? Non controlli gli input. È intenzionale, cioè ti affidi a GetBytes per gettare ArgumentNullException al tuo posto? È un errore?
-
Che cosa succede se plainText è lungo e, a lungo, intendo il massimo consentito da Windows? Questo è un caso complicato, dal momento che il test delle unità non è ovvio (anche in termini di prestazioni e consumo di memoria: nessuno vuole che i test unitari tengano tutta la memoria disponibile), ma dovresti comunque considerarlo.
Suggerimento: il modo più semplice e veloce per gestire questo caso è specificare la lunghezza massima supportata nei requisiti del prodotto e quindi applicare questo limite al passo di convalida dell'input.
-
Che cosa succede se plainText è una stringa vuota?
-
Che cosa succede se chiami il metodo un sacco di volte? Non si dispone di SHA256CryptoServiceProvider , nonostante il fatto che si dovrebbe, il che significa che prima o poi, fallirà più o meno imprevedibilmente. Questo è un altro caso in cui il test unitario è complicato / praticamente impossibile.
In secondo luogo, hai ragione, puoi fare affidamento sui metodi di .NET Framework e assumere che siano implementati correttamente. Ciò significa che devi testare la tua logica e il modo in cui usi questi metodi:
-
Scrivi un semplice test che chiama il metodo con "Hello World!" come input. Prendi qualsiasi linguaggio non- .NET: JavaScript, Python, qualunque cosa. Calcola l'hash SHA256 con questa lingua e inserisci il risultato hard nel test dell'unità. Confronta.
-
Analizza il tuo codice (nota: in TDD, non lo fai: scrivi test dai casi d'uso, non dal codice reale). Tale analisi dovrebbe darti la visione generale del flusso: dove sono le condizioni, come esci dai loop, ci sono clausole di guardia, ecc. Il flusso reale è semplice: una sequenza di chiamate a metodi diversi, quindi una for . A parte il using mancante, la sequenza di chiamate può creare il rischio di utilizzare una variabile mentre è null . Ad esempio:
var product = this.GetProductById(productId);
var category = product.GetCategory();
è sbagliato, a meno che non ci sia una garanzia che GetProductById non possa restituire null . La precedente parte di codice avrebbe dovuto essere:
var product = this.GetProductById(productId);
if (product == null)
{
// Handle the situation and either throw an exception or prematurely return/break.
}
var category = product.GetCategory();
o
var product = this.GetProductById(productId);
Debug.Assert(
product != null,
"The null result is unexpected from ProductsManager.GetProductById method.");
var category = product.GetCategory();
o
var product = this.GetProductById(productId);
Contract.Assume(product != null);
var category = product.GetCategory();
Nel tuo caso, ci sono solo costruttori, quindi non devi preoccuparti dei riferimenti null.
for che usi erroneamente (dovrebbe essere foreach ) è un altro punto da considerare. Il ciclo è fatto correttamente? Devi fare i < hashedBytes.Length o i <= hashedBytes.Length ? Quando accedi a hashedBytes per indice, sei certo che l'indice non sia mai fuori portata? La lunghezza potrebbe essere maggiore di int.MaxValue ? Cosa succederebbe?
Infine, stai utilizzando sia byte.ToString che ToLower senza specificare la cultura. Dal momento che non dici né che ti aspetti che venga utilizzato InvariantCulture , potrebbe indicare che hai semplicemente dimenticato di indicare chiaramente il tuo intento. Entrambi i metodi usano la cultura corrente. Ciò significa che devi anche eseguire dei test unitari in cui la cultura corrente non è en-US , poiché il risultato finale del tuo metodo SHA256 dipende da esso (o utilizzare metodi formali per dimostrare che il risultato sarebbe lo stesso per qualsiasi cultura ).
Inoltre, byte.ToString("x2") produce già una stringa minuscola. La conversione di una stringa minuscola in minuscolo non è necessaria. Rimuovi sempre il codice che non ti serve.
Conclusione:
La tua ipotesi di poter contare su metodi di .NET Framework è vera. Tuttavia, il modo in cui li usi dovrebbe essere testato su unità. Come puoi vedere, in questo metodo molto breve con logica semplice ,
-
esiste un potenziale bug grave che verrà visualizzato prima o poi quando qualcuno inizierà a modificare questo metodo.
-
non disponendo gli oggetti correttamente, ha creato un bug che sarà molto difficile eseguire il debug in seguito ,
-
lo ha reso incline agli errori utilizzando in modo errato for quando era previsto foreach ,
-
l'ha reso potenzialmente vulnerabile non controllando i tuoi input (mentre questo è un metodo pubblico),
-
ha reso difficile indovina quali eccezioni possono essere generate da questo metodo,
-
ha reso il suo comportamento molto casuale quando si tratta di casi limite (stringhe troppo lunghe, null , ecc.)
Avresti potuto evitare alcuni di questi problemi con test severi, ma il test da solo non è sufficiente. Usare altre tecniche, come revisioni informali del codice o controllo statico dovrebbe essere sistematico e coprire se non tutto, almeno la maggior parte del codice non CRUD che scrivi.