I chiamanti devono controllare la validità degli argomenti prima di chiamare il costruttore?

Question

Recentemente ho letto molto su TDD e codice pulito, quindi ho iniziato a lavorare su un progetto semplice che li mette in pratica e mi sono imbattuto in qualcosa che non sono sicuro di quale sia l'approccio migliore da adottare.

Ho una classe che accetta un oggetto Java File come parametro, l'aspettativa è che questo oggetto File deve essere una directory e deve iniziare con un certo prefisso. Il mio primo passaggio consisteva nel fare controlli sull'oggetto File prima di chiamare il costruttore, controllando che fosse una directory e controllando che il nome fosse valido. Ma non mi piace che sia il chiamante che sta specificando ciò che lo rende valido e in particolare quello che è il prefisso valido, penso che questa logica dovrebbe essere collocata nella classe stessa.

Potrei fare questo controllo nel costruttore e lanciare un'eccezione se non è valida, ma data la natura del problema, se sto iterando su un elenco di File s allora è del tutto previsto che alcuni di loro hanno vinto 'essere' valido '(vale a dire che saranno file piuttosto che directory), quindi è un lancio di un Exception davvero giustificato?

public MyObject(File directory) { 
    if (!directory.isDirectory()) { 
     throw new IllegalArgumentException("Must be a directory"); 
    } 
    if (!directory.getName().startsWith("Prefix")) { 
     throw new IllegalArgumentException("Must start with Prefix"); 
    } 
    .... 
} 

ho pensato di aggiungere forse un metodo factory per creare gli oggetti e restituendo null se l'File non è valido.

public static MyObject createMyObject(File directory) { 
    if (!directory.isDirectory() || !directory.getName().startsWith("Prefix")) { 
     return null; 
    } 
    return new MyObject(directory); 
} 

In alternativa ho pensato di aggiungere un metodo statico alla classe che convalida il file per il chiamante prima di chiamare il costruttore.

public static boolean isValid(File directory) { 
    return directory.isDirectory() && directory.getName().startsWith("Prefix"); 
} 

if (MyObject.isValid(directory)) { 
    MyObject object = new MyObject(directory); 
} 

Quindi, in termini di codice pulito e di tutti i principi di programmazione orientata agli oggetti (come la singola responsabilità, accoppiamento, ecc), che sarebbe il modo migliore di fare questo?

UPDATE:

Avendo letto alcune delle risposte che sono state inserite già ho cominciato a pensare a un'altra possibilità che le sarebbero applicabili solo la mia situazione attuale, piuttosto che genericamente come la mia domanda era veramente.

Come parte del mio codice chiamante ho un percorso dal filesystem e sto elencando tutti i file in quella directory ed è ogni file che sto passando al costruttore MyObject se è valido o meno. Potrei passare un FileFilter al metodo listFiles che garantisce che listFiles restituisca solo directory valide. Il FileFilter poteva essere dichiarato entro MyObject:

public static FileFilter getFilter() { 
    return new FileFilter() { 
     public boolean accept(File path) { 
      return path.isDirectory() && path.getName().startsWith("Prefix"); 
     } 
    }; 
} 

Se il mio costruttore ha generato un'eccezione allora sarebbe davvero essere una situazione eccezionale, perché l'aspettativa è che è solo stato passato directory valide. Fare questo significherebbe che potrei rimuovere la necessità di un'eccezione controllata da costruttore/factory poiché qualsiasi eccezione indicherà un bug da qualche parte piuttosto che un comportamento previsto. Ma lascia ancora la questione se metterlo in un costruttore o in un metodo di fabbrica.

Answer 1

Sarebbe la mia preferenza offrire la combinazione di un costruttore che convalida e genera un'eccezione quando l'argomento indicato non completa il suo contratto completato dal metodo di convalida static boolean isValid().

Utilizzato in un ciclo, il metodo di convalida offre un modo piacevole e leggibile di costruire oggetti validi, mentre il costruttore si assicura che gli usi non convalidati vengano gestiti generando un'eccezione quando necessario.

Answer 2

public static MyObject createMyObject(File directory) throws IllegalArgumentException{ 
    if (!directory.isDirectory() || !directory.getName().startsWith("Prefix")) { 
     return throw new IllegalArgumentException("invalid parameters")"; 
    } 
    return new MyObject(directory); 
} 

Questa è una delle opzioni che è una combinazione di due delle opzioni suggerite. Utilizza il metodo Factory e convalida anche le pre-condizioni, che è ciò che i metodi di fabbrica sono in grado di fare. Quindi IMO questa può essere una buona opzione.

Perché non l'opzione 2 in quanto è: Perché tornare nulls è una cattiva scelta quando si può generare eccezioni per avvertire l'utente che alcune pre-condizioni non sono state soddisfatte.

UPDATE: Questa strategia dà garanzia che sarà creato solo un oggetto in stato valido. Inoltre, se si vuole prendere in giro l'istanza di MyObject, è possibile implementare un'interfaccia per l'utilizzo del polimorfismo di runtime e distribuire gli oggetti fittizi. Spero che abbia un senso.

Answer 3

Penso che la posizione del codice di convalida dipenda esattamente da cosa rappresenta la classe "MyObject".

Se MyObject esegue alcune operazioni che fallirebbero se avesse un file piuttosto che una directory, allora direi che dovrebbe contenere il codice di convalida nel suo costruttore - ciò rende la classe autonoma e consentirà la possibile uso della classe più tardi.

Se MyObject è solo un contenitore per il file/directory, e non esiste un codice specifico per directory in esso, quindi inserire il codice di validazione all'interno di quella classe che fa richiedono una directory piuttosto che un file.

Answer 4

Dipende ...

Dal punto di vista di codifica, l'approccio più semplice e più pulito è il costruttore che esplode (unchdcked). Se vi è una ragionevole aspettativa che il chiamante debba e debba solo passare le directory, procedere con quello.

Se v'è la ragionevole aspettativa che il chiamante può passare in non-directory, quindi si hanno due scelte:

1. Se il chiamante può affrontare la situazione, hanno la funzione di costruzione gettare un controllato eccezione
2. Se il chiamante non può gestire un'eccezione, è possibile che la classe "non faccia nulla" se il chiamante invoca un metodo, ignorando sostanzialmente tutte le richieste di "eseguire" qualcosa se il file passato al costruttore non è una directory.

Answer 5

Questo è chiamato Design-by-Contract e ci sono librerie per, per aiutarti a controllare gli argomenti in entrata.