La convalida dei campi sia nel costruttore che nel setter è considerata un codice ridondante errato?

Question

Ho la seguente classe:

public class Project { 

    private int id; 
    private String name; 

    public Project(int id, String name) { 
     if(name == null){ 
      throw new NullPointerException("Name can't be null"); 
     } 

     if(id == 0){ 
      throw new IllegalArgumentException("id can't be zero"); 
     } 

      this.name = name; 
      this.id = id; 

    } 

    private Project(){} 

    public int getId() { 
     return id; 
    } 

    public void setId(int id) { 
     if(id == 0){ 
      throw new IllegalArgumentException("id can't be zero"); 
     } 
     this.id = id; 
    } 

    public String getName() 
     return name; 
    } 

    public void setName(String name) { 
     if(name == null){ 
      throw new NullPointerException("Name can't be null"); 
     } 
     this.name = name; 
    } 

} 

Se avete notato che setName e setId condividono la stessa convalida per i suoi campi con il costruttore. È questo codice ridondante che potrebbe causare problemi in futuro (ad esempio se qualcuno modifica il setter per consentire 0 per l'id e prevenire -1 invece di cambiare il costruttore)? . Dovrei usare un metodo privato per fare il check e condividerlo tra il costruttore e il setter che sembra troppo se ci sono molti campi.

Nota: questo è il motivo per cui non utilizzo i setter nel costruttore. https://stackoverflow.com/a/4893604/302707

Answer 1

Si consiglia di definire un metodo per campo come e quindi chiamare lo stesso metodo in Setter e Costruttore.

Answer 2

Direi di si. Piuttosto che quello, basta chiamare il setter dal costruttore:

public Project(int id, String name, Date creationDate, int fps, List<String> frames) { 
    setName(name); 
    setId(id); 
    // other stuff with creationDate, fps and frames? 
} 

Inoltre, non si dovrebbe verificare per un nulla name in getName - farlo in setName. In caso contrario, i bug saranno difficili da rintracciare - si desidera rilevare il nome non valido non appena viene utilizzato, non quando viene utilizzato (che potrebbe essere molto più tardi).

Answer 3

se si rende immutabile Project, si eliminerà il codice ridondante. ma per ora, penso che il lancio esplicito di eccezioni sia nel metodo costruttore che in quello mutatore va bene.

e non chiamerei i metodi del mutatore all'interno del costruttore per molte ragioni, tra cui this. inoltre, vorrei rimuovere il codice di convalida nel metodo accessor ... non è necessario.

Answer 4

ecco il codice rivisto:

public class Project { 

    private int id; 
    private String name; 

    public Project(int id, String name, Date creationDate, int fps, List<String> frames) { 

     checkId(id); 
      checkName(name); 
      //Insted of lines above you can call setters too. 

      this.name = name; 
      this.id = id; 

    } 

    private Project(){} 

    public int getId() { 
     return id; 
    } 

    public void setId(int id) { 
     checkId(id); 
     this.id = id; 
    } 

    public String getName() 
     return name; 
    } 

    public void setName(String name) { 
      checkName(name); 
     this.name = name; 
    } 

    private void checkId(int id){ 
     if(id == 0){ 
      throw new IllegalArgumentException("id can't be zero"); 
     } 
    } 

    private void checkName(String name){ 
      if(name == null){ 
      throw new NullPointerException("Name can't be null"); 
     } 
    } 

}