正在验证构造函数和setter中的字段，这被认为是错误的冗余代码

Question

我有以下类：

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

如果您注意到setName和setId与构造函数共享对其字段的相同验证。这是不是多余的代码可能会在未来引发问题（例如，如果有人编辑setter以允许id为0，而阻止-1，但没有更改构造函数）。我应该使用一个私有方法来进行检查，并在构造函数和setter之间共享它吗？如果有很多字段，这似乎太多了。

注意：这就是为什么我不在构造函数中使用setter的原因。https://stackoverflow.com/a/4893604/302707

Answer 1

这是修改后的代码：

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

Answer 2

我建议您应该将每个字段定义一个方法为isValid()，然后在setter和Constructor中调用相同的方法。

Answer 3

我会答应的。与其这样，只需从构造函数中调用setter：

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

此外，您不应该在getName中检查空的name，而是在setName中检查。否则，错误将很难追踪——您希望在无效名称出现时立即捕获，而不是在使用时（可能要晚得多）。

Answer 4

如果您使Project不可变，它将消除多余的代码。但就目前而言，我认为在构造函数和赋值函数方法中显式抛出异常是可以的。

由于许多原因，我不会在构造函数中调用mutator方法，包括这一点。此外，我将删除访问器方法中的验证代码。。。没有必要。