Java Constructor of a class with fields that are are end user fed

Question

My question is: What is the best way to write the constructor of a Java class with fields that are initialized through stdin?

For example suppose that I have an Employee class that looks like:

Public class Employee {
    private int empID;
    private String empName;
    private List<Role> empRoles;
    {....}
}

I can write all the setters and getters for this class. Of course, the Role class will have its own file.

Also suppose that I make my setters for the first two fields as follows, in order to enable the end user to initialize the fields:

public void setEmpID() {
    System.out.println("Please enter the employee ID");
    Scanner s = new Scanner (System.in);
    this.empID = s.nextInt();

public void setEmpName() {
    System.out.println("Please enter the employee name");
    Scanner s = new Scanner (System.in);
    this.empName = s.next();
}

then:

1. Can I use such setters in a constructor that overrides the default constructor.
2. Is this the best way to write such constructors?
3. Is it better to move the Scanner object I am creating in each setter to the constructor and make it as an argument for the setters

e.g:

public void setEmpName(Scanner s) {
    ...
    this.empName = s.next();
}

As you can see, this may be a design question rather than just "coding".

Many thanks for your help.

Answer 1

There are two good answers already but I want to give you one more solution to your problem. As @davidxx already said if your object should be immutable all arguments constructor is a better aproach instead of setters but let's think for the case where you have more fields. For example your employee have salary, experience and other. Your constructor starts to look like this:

Employee employee = new Employee(id, name, roles, salary, experience, ... );

As you can see the constructor starts becoming too long. This is called telescoping constructor. Let's think for the case where your employee has 2-3 required fields and the others are not required. To create this object you'll have to write code like this:

Employee employee = new Employee(id, name, roles, null, null, 0, ... );

This is problematic because :

1. Passing null to functions may caused you a lot of headaches.

2. This code is not so readable.

You can add constructor which recieves only fields that you need but then you'll have to add a new constructor(breaking Open-Closed principle) every time when you need to pass a different combination of parameters. Solution for this situation is to use the builder pattern:

public class Employee {
    private int id;
    private String name;
    private List<Role> roles;

    private Employee() {
        roles = new ArrayList<>();
    }

    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public List<Role> getRoles() {
        return roles;
    }

    public static class EmployeeBuilder {

        private Employee employee;

        public EmployeeBuilder() {
            employee = new Employee();
        }

        public EmployeeBuilder withId(Integer id) {
            employee.id = id;
            return this;
        }

        public EmployeeBuilder withName(String name) {
            employee.name = name;
            return this;
        }

        public EmployeeBuilder withRole(Role role) {
            employee.roles.add(role);
            return this;
        }

        public Employee build() {
            return employee;
        }

    }
}

And then you can create your object like this:

Employee employee = new Employee.EmployeeBuilder()
                    .withId(1)
                    .withName("John")
                    .withRole(role1)
                    .withRole(role2)
                    .build();

Answer 2

I think that you may be confusing user input/output with program model. The key here is that you should keep the two completely separate. The Employee class should have absolutely no knowledge about what type of UI or I/O is being done to use it, since in this way it can be used in a GUI, in a console program or anywhere else it is needed.

So your Employee constructors should just take in the data needed to create an Employee object, irrespective of its source, and the same for your field getters.

So your getters will look nothing like you've posted and instead be much more plain, much more "dumb" or "ignorant" of user I/O (Scanner, System.in, and the like)

public void getEmpID (int empID) {
    this.empID = empID;
}

same for the other fields.

All the I/O stuff -- the Scanner class and such goes elsewhere in your driver class.

Side note: when you use a Scanner based on the System.in your program should create one and only one such beast, create it when needed, and then close and dispose of it only when the program is completely done with it. Otherwise you risk breaking system input by prematurely closing the connection. This is yet another reason not to use your proposed code where you create multiple Scanner objects.

For example....

import java.util.ArrayList;
import java.util.List;

public class Employee {
    private int empID;
    private String empName;
    private List<Role> empRoles;

    public Employee(int empID, String empName) {
        super();
        this.empID = empID;
        this.empName = empName;
        empRoles = new ArrayList<>();
    }

    public int getEmpID() {
        return empID;
    }

    public void setEmpID(int empID) {
        this.empID = empID;
    }

    public String getEmpName() {
        return empName;
    }

    public void setEmpName(String empName) {
        this.empName = empName;
    }

    public List<Role> getEmpRoles() {
        return empRoles;
    }

    public boolean addEmpRole(Role role) {
        return empRoles.add(role);
    }

    public boolean removeEmpRole(Role role) {
        return empRoles.remove(role);
    }

}

You can then use it elsewhere like so:

import java.util.Scanner;

public class TestEmployee {
    public static void main(String[] args) {
        Scanner scan = new Scanner(System.in);

        System.out.print("Enter employee ID: ");
        int empID = scan.nextInt();
        scan.nextLine();  // handle dangling end of line token

        System.out.print("Enter employee Name: ");
        String empName = scan.nextLine();
        Employee employee = new Employee(empID, empName);       

        // if we are **totally** done with the Scanner, now we may close it
        scan.close();
    }
}

Answer 3

Actually, you don't rely on a way that uses a specific constructor to populate fields of the object but a no arg constructor.
You indeed chose a setter approach to populate fields of the Employee instance after invoking new Employe().
But this setter approach is complex as you mix too many responsibilities : taking user input and setting state of the object.

Can I use such setters in a constructor that overrides the default constructor.

No, it makes no sense : constructor and setters are two distinct ways and you cannot override one with the other.
You could however invoke setters from the constructor by relying on a Scanner instance to take user input but similarly to your actual setters approach, it seems a awkward approach as it gives too many responsibilities to the constructor.

Is this the best way to write such constructors?

Using a constructor that populates all fields, that is :

Employee emp = new Employe(id, name, roles)

makes sense if your object is designed to be immutable once it is created.

In your actual case, if your object is not designed to be immutable using constructor or setters is valid but in any case, you should provide setters.

---

So to answer to your question, you should separate responsibilities (taking user input and setting the object state) and use either setter or constructor approach according to your requirements on instances of Employee :

Employee emp = new Employe(id, name, roles)

or

Employee emp = new Employe();
emp.setId(...);
emp.setName(...);
emp.setRoles(...);