Constructor vs setter validations

Question

I have the following class :

public class Project {

    private int id;
    private String name;  

    public Project(int id, String name, Date creationDate, int fps, List<String> frames) {
        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) { 
        this.id = id;
    }

    public String getName() {
        return name;
    }

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

}

I have three questions:

1. Do I use the class setters instead of setting the fields directly. One of the reason that I set it directly, is that in the code the setters are not final and they could be overridden.

2. If the right way is to set it directly and I want to make sure that the name filed is not null always. Should I provide two checks, one in the constructor and one in the setter.

3. I read in effective java that I should use NullPointerException for null parameters. Should I use IllegalArgumentException for other checks, like id in the example.

Answer 1

Your class allows someone to say myProject.setName(null), avoiding the null check in the constructor. If you want to set stuff through the constructor but need checks (question 2), I suggest doing the checks in set-methods and call those from the constructor. Example:

public class Project {

    private int id;
    private String name;  

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

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

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

}

This avoids code duplication.

Answer 2

I believe you shouldn't get anything extra in the getters and setters. There will be time in the future where putting extra functionalities in getters and setters cause weird side effects.

But the most compelling reason is when you use the class as data entity, those getters and setters might be used by java bean container (EJB, Spring), JPA, XML marshaller and unmarshaller.

I also prefer using IllegalArgumentException over NPE. If you plan to catch and recover or just log the error, it's almost always better to be as specific as possible.

But as you mentioned already, for validation. Having a separate validation method is much better.