Validazione del parametro di input nel chiamante: duplicazione del codice?

Dipende. Decidere dove mettere la convalida dovrebbe essere basata sulla descrizione e forza del contratto implicita (o documentata) dal metodo. La validazione è un buon modo per rafforzare l'adesione a un contratto specifico. Se per qualsiasi ragione il metodo ha un contratto molto rigido, allora sì, spetta a te controllare prima di chiamare.

Questo è un concetto particolarmente importante quando crei un metodo pubblico , perché in pratica stai facendo pubblicità che alcuni metodi eseguono alcune operazioni. È meglio fare ciò che dici di fare!

Adotta il seguente metodo come esempio:

public void DeletePerson(Person p)
{            
    _database.Delete(p);
}

Qual è il contratto implicito da DeletePerson ? Il programmatore può solo presumere che se viene passato qualche Person , verrà eliminato. Tuttavia, sappiamo che questo non è sempre vero. Cosa succede se p è un valore null ? Cosa succede se p non esiste nel database? Cosa succede se il database è disconnesso? Pertanto, DeletePerson non sembra soddisfare correttamente il suo contratto. a volte elimina una persona, a volte genera una NullReferenceException o una DatabaseNotConnectedException o, a volte, non fa nulla (come se la persona fosse già cancellato).

API come questa sono notoriamente difficili da usare, perché quando chiami questa "scatola nera" di un metodo, possono accadere cose terribili.

Ecco un paio di modi in cui puoi migliorare il contratto:

• Aggiungi la convalida e aggiungi un'eccezione al contratto. Ciò rende il contratto più strong , ma richiede che il chiamante esegua la convalida. La differenza, tuttavia, è che ora conoscono le loro esigenze. In questo caso, comunico questo con un commento XML C #, ma potresti invece aggiungere un throws (Java), usare un Assert o usare uno strumento di contratto come Contratti di codice.

  ///<exception>ArgumentNullException</exception>
  ///<exception>ArgumentException</exception>
  public void DeletePerson(Person p)
  {            
      if(p == null)
          throw new ArgumentNullException("p");
      if(!_database.Contains(p))
          throw new ArgumentException("The Person specified is not in the database.");
  
      _database.Delete(p);
  }
  

  _{Nota a margine: l'argomento contro questo stile è spesso che causa un'eccessiva pre-convalida da parte di tutto il codice chiamante, ma nella mia esperienza questo spesso non è il caso. Pensa a uno scenario in cui stai tentando di eliminare una Persona nullo. Come è successo? Da dove viene la Persona nulla? Ad esempio, se si tratta di un'interfaccia utente, perché la chiave di cancellazione è stata gestita se non è stata selezionata la selezione corrente? Se fosse già stato cancellato, non dovrebbe essere già stato rimosso dal display? Ovviamente ci sono delle eccezioni a questo, ma man mano che un progetto cresce, ringrazierai spesso codice come questo per evitare che i bachi penetrino nel sistema.}

• Aggiungi validazione e codice in modo difensivo. Questo rende looser di contratto, perché ora questo metodo fa molto più che eliminare la persona. Ho cambiato il nome del metodo per riflettere questo, ma potrebbe non essere necessario se sei coerente nella tua API. Questo approccio ha i suoi pro e contro. Il pro è che ora puoi chiamare TryDeletePerson passando in tutti i tipi di input non validi e non preoccuparti mai delle eccezioni. Il problema, ovviamente, è che gli utenti del tuo codice probabilmente chiameranno questo metodo troppo o potrebbero rendere difficile il debug nei casi in cui p è nullo. Questa potrebbe essere considerata una lieve violazione del principio di responsabilità singola , quindi tienilo a mente se scoppia una guerra di fuoco.

  public void TryDeletePerson(Person p)
  {            
      if(p == null || !_database.Contains(p))
          return;
  
      _database.Delete(p);
  }
  

• Combina approcci. A volte vuoi un po 'di entrambi, dove vuoi che i chiamanti esterni seguano le regole da vicino (per costringerli al codice responsabile), ma vuoi che il tuo codice privato sia flessibile.

  ///<exception>ArgumentNullException</exception>
  ///<exception>ArgumentException</exception>
  public void DeletePerson(Person p)
  {            
      if(p == null)
          throw new ArgumentNullException("p");
      if(!_database.Contains(p))
          throw new ArgumentException("The Person specified is not in the database.");
  
      TryDeletePerson(p);
  }
  
  internal void TryDeletePerson(Person p)
  {            
      if(p == null || !_database.Contains(p))
          return;
  
      _database.Delete(p);
  }
  

In base alla mia esperienza, concentrarmi sui contratti che intendi piuttosto che su una dura regola funziona meglio. La codifica difensiva sembra funzionare meglio nei casi in cui è difficile o difficile per il chiamante determinare se un'operazione è valida. I contratti rigorosi sembrano funzionare meglio dove ci si aspetta che il chiamante effettui solo chiamate di metodo quando hanno davvero, davvero un senso.

È una questione di convenzione, documentazione e casi d'uso.

Non tutte le funzioni sono uguali. Non tutti i requisiti sono uguali. Non tutte le convalide sono uguali.

Ad esempio, se il tuo progetto Java cerca di evitare puntatori nulli quando possibile (vedi i consigli Guava style , ad esempio), convalidi ancora ogni argomento di funzione per assicurarti che non sia nullo? Probabilmente non è necessario, ma è probabile che lo fai ancora, per rendere più facile trovare bug. Ma puoi usare un assert dove hai precedentemente gettato una NullPointerException.

Cosa succede se il progetto è in C ++? La convenzione / tradizione in C ++ serve per documentare le precondizioni, ma solo verificarle (se lo sono) in build di debug.

In entrambi i casi, hai una precondizione documentata sulla tua funzione: nessun argomento può essere nullo. Potresti invece estendere il dominio della funzione per includere valori null con comportamento definito, ad es. "se qualsiasi argomento è nullo, genera un'eccezione". Ovviamente, è di nuovo il mio retaggio C ++ che parla qui - in Java, è abbastanza comune documentare le precondizioni in questo modo.

Ma non tutte le precondizioni possono essere ragionevolmente controllate. Ad esempio, un algoritmo di ricerca binaria ha la precondizione che la sequenza da cercare deve essere ordinata. Ma verificare che sia così è un'operazione di O (N), quindi farlo su ogni tipo di chiamata sconfigge il punto di usare un algoritmo di O (log (N)) in primo luogo. Se si sta programmando in modo difensivo, è possibile eseguire controlli minori (ad esempio, verificando che per ogni partizione in cui si esegue la ricerca, i valori iniziale, intermedio e finale siano ordinati), ma che non rilevi tutti gli errori. In genere, devi solo fare affidamento sul fatto che la condizione sia soddisfatta.

L'unico posto reale in cui hai bisogno di controlli espliciti è ai limiti. Input esterno al tuo progetto? Convalida, convalida, convalida. Un'area grigia è costituita dai confini dell'API. Dipende molto da quanto si vuole fidarsi del codice client, da quanto danno fa l'input non valido e da quanta assistenza si vuole fornire nel trovare bug. Ogni limite di privilegi deve essere considerato come esterno, ovviamente - le syscalls, ad esempio, vengono eseguite in un contesto di privilegi elevato e quindi devono essere molto attenti a convalidare. Qualsiasi convalida di questo tipo deve ovviamente essere interna a syscall.

La convalida dei parametri dovrebbe essere la preoccupazione della funzione chiamata. La funzione dovrebbe sapere cosa è considerato input valido e cosa no. I chiamanti potrebbero non saperlo, soprattutto quando non sanno come la funzione è implementata internamente. Ci si aspetta che la funzione gestisca qualsiasi combinazione di valori dei parametri dai chiamanti.

Poiché la funzione è responsabile della convalida dei parametri, è possibile scrivere test di unità con questa funzione per assicurarsi che si comporti come previsto con valori di parametri validi e non validi.

All'interno della funzione stessa. Se la funzione viene utilizzata più di una volta, non si desidera verificare il parametro per ogni chiamata di funzione.

Inoltre, se la funzione viene aggiornata in modo tale da influire sulla convalida del parametro, è necessario cercare ogni occorrenza della convalida del chiamante per aggiornarli. Non è adorabile :-).

Puoi fare riferimento a Clausola di salvaguardia

Aggiorna

Vedi la mia risposta per ogni scenario che hai fornito.

• quando il trattamento della variabile non valida può variare, è bene convalidarlo dal lato del chiamante (es. funzione sqrt() - in alcuni casi potrei voler lavorare con un numero complesso, quindi considero la condizione nel chiamante)

  risposta

  La maggior parte dei linguaggi di programmazione supporta numeri interi e reali per impostazione predefinita, non numero complesso, quindi la loro implementazione di sqrt accetta solo numeri non negativi. L'unico caso in cui hai una funzione sqrt che restituisce un numero complesso è quando usi un linguaggio di programmazione orientato alla matematica, come Mathematica

  Inoltre, sqrt per la maggior parte dei linguaggi di programmazione è già implementato, quindi non è possibile modificarlo, e se provi a sostituire l'implementazione (vedi patch di scimmia), i tuoi collaboratori rimarranno delusi sul perché sqrt improvvisamente accetta numeri negativi.

  Se ne volevi uno puoi avvolgerlo attorno alla tua funzione sqrt personalizzata che gestisce il numero negativo e restituisce un numero complesso.

• quando la condizione di controllo è la stessa in ogni chiamante, è meglio controllarla all'interno della funzione, per evitare duplicazioni

  risposta

  Sì, è una buona pratica evitare di spargere la convalida dei parametri nel codice.

• la convalida del parametro di ingresso nel chiamante avviene solo prima di chiamare molte funzioni con questo parametro. Pertanto la convalida di un parametro in ciascuna funzione non è efficace

  risposta

  Sarà bello se il chiamante è una funzione, non credi?

  Se le funzioni all'interno del chiamante sono utilizzate da un altro chiamante, cosa ti impedisce di convalidare il parametro all'interno delle funzioni chiamate dal chiamante?

• la soluzione giusta dipende dal caso particolare

  risposta

  Punta a codice manutenibile. Spostare la convalida dei parametri garantisce una fonte di verità su ciò che la funzione può accettare o meno.

Una funzione dovrebbe indicare le sue condizioni pre e post.
Le pre-condizioni sono le condizioni che devono essere soddisfatte dal chiamante prima di poter utilizzare correttamente la funzione e possono (e spesso fanno) includere la validità dei parametri di input.
Le post-condizioni sono le promesse fatte dalla funzione ai suoi chiamanti.

Quando la validità dei parametri di una funzione è parte delle pre-condizioni, è responsabilità del chiamante assicurarsi che quei parametri siano validi. Ma ciò non significa che ogni chiamante debba controllare esplicitamente ciascun parametro prima della chiamata. Nella maggior parte dei casi, non sono necessari test espliciti perché la logica interna e le pre-condizioni del chiamante assicurano già che i parametri siano validi.

Come misura di sicurezza contro errori di programmazione (bug), puoi verificare che i parametri passati a una funzione soddisfino realmente le condizioni preliminari indicate. Poiché questi test possono essere costosi, è una buona idea essere in grado di disattivarli per le build di rilascio. Se questi test falliscono, allora il programma dovrebbe essere terminato, perché si è verificato in un errore.

Anche se a prima vista il check-in del chiamante sembra invitare la duplicazione del codice, in realtà è il contrario. Il check in the callee comporta la duplicazione del codice e molti lavori non necessari.
Basta pensarci, quanto spesso si passano i parametri attraverso diversi livelli di funzioni, apportando solo piccole modifiche ad alcuni di essi lungo il percorso. Se applicate costantemente il metodo check-in-callee , ciascuna di queste funzioni intermedie dovrà ripetere il controllo per ciascuno dei parametri.
E ora immagina che uno di quei parametri debba essere una lista ordinata.
Con il check-in del chiamante, solo la prima funzione dovrebbe assicurarsi che l'elenco sia realmente ordinato. Tutti gli altri sanno che l'elenco è già stato ordinato (come è quello che hanno dichiarato nella loro pre-condizione) e può passarlo senza ulteriori verifiche.

Molto spesso non puoi sapere chi, quando e come chiamerà la funzione che hai scritto. È meglio assumere il peggio: la funzione verrà chiamata con parametri non validi. Quindi dovresti assolutamente coprirlo.

Tuttavia, se la lingua che usi supporta le eccezioni, potresti non controllare alcuni errori e accertarti che venga lanciata un'eccezione, ma in questo caso devi essere sicuro di descrivere il caso nella documentazione (devi avere documentazione). L'eccezione fornirà al chiamante informazioni sufficienti su ciò che è successo e attirerà anche l'attenzione sugli argomenti non validi.