¿Validar campos tanto en el constructor como en el setter es considerado un código redundante malo?

Question

que tienen la clase siguiente:

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; 
    } 

} 

Si notas que setName y setId comparten la misma validación de sus campos con el constructor. ¿Este código redundante podría causar problemas en el futuro (por ejemplo, si alguien edita el setter para permitir 0 para el id y evitar -1 en su lugar, pero no cambió el constructor)? . ¿Debo usar un método privado para hacer el control y compartirlo entre el constructor y el colocador, que parece demasiado si hay muchos campos?

Nota: Esta es la razón por la que no estoy usando los setters en el constructor. https://stackoverflow.com/a/4893604/302707

Answer 1

Recomiendo que debe definir un método por campo como isValid() y luego llamar al mismo método en setter y en Constructor.

Answer 2

Yo diría que sí. En lugar de eso, simplemente llame a los emisores de su constructor:

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

Además, no se debe comprobar si hay un nulo name en getName - lo hacen en setName. De lo contrario, los bugs serán difíciles de rastrear; querrás capturar el nombre inválido tan pronto como ingrese, no cuando se use (lo cual puede ser mucho más tarde).

Answer 3

si hace que Project sea inmutable, eliminará el código redundante. pero por ahora, creo que arrojar excepciones explícitamente en los métodos constructor y mutador está bien.

y no llamaría a los métodos de mutador dentro del constructor por muchas razones, incluyendo this. Además, eliminaría el código de validación en el método de acceso ... no es necesario.

Answer 4

Aquí está el código revisado:

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"); 
     } 
    } 

}