Geek asks and answers

Is checking fields in both constructors and setters bad redundant code?

I have the following class:

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

}

If you notice that setName and setId use the same check for their constructor fields. Is this redundant code that may cause problems in the future (for example, if someone edits the installer to allow 0 for id and instead prevent -1, but did not change the constructor)? Should I use a private method to check and share it between the designer and the installer, which seems too big if there are many fields.

Note. That is why I do not use setters in the constructor. stack overflow

+5
source share
4 answers

:

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

}
+2

isValid(), , .

+1

, . :

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

, null name getName - setName. - , , ( ).

0

if you make it Projectimmutable, it will eliminate redundant code. but for now, I think that explicitly throwing exceptions in both the constructor method and the mutator method is fine.

and I would not name the mutator methods inside the constructor for many reasons, including this . also, I would delete the verification code in the access method ... it is not needed.

0
source

More articles:


All Articles