Creating Immutable Student Object

Question

For a project I'm working on I have the following requirements:

We have two student types: Domestic and International

public enum StudentType {
    Domestic,International
}

Students of both types must have a ID number, First/Last name but only international students require documentation (Passports, drivers licence, etc).

I've decided to create the Student class as immutable, and came up with the following implementation:

public final class Student {

    private final String id;
    private final String firstname;
    private final String lastname;
    private final Collection<String> documents;;
    private final StudentType typeofStudent;

    private Student(String id, String firstname, String lastname, List<String> documents, StudentType typeofStudent)
    {   
        this.id = id;
        this.firstname = firstname;
        this.lastname = lastname;
        this.documents = Collections.unmodifiableList(new ArrayList<String>(documents));
        this.typeofStudent = typeofStudent;
    }

    public static Student createDomestic(String id, String firstname, String lastname)
    {
        return new Student(id, firstname, lastname, Collections.emptyList(), StudentType.Domestic);
    }

    public static Student createInternational(String id, String firstname, String lastname, List<String> documents) 
    {   
        return new Student(id, firstname, lastname, documents, StudentType.International);
    }
} 

I left out constructor validation to keep my code short and concise.

Advantages of the static method approach:

• If the constructor was public, I would need to write a private helper method that checks if the student is domestic, the client didn't pass a list of documents. I would throw an exception to notify the client if this happened.
• My code is shorter because I prohibit the client from passing a List<String> documents if the student was domestic. I simply ask for the ID, and first/last name. The Collections.unmodifiableList(); and StudentType are set when domestic students are created. In other words, no need for a private helper method.
• The client knows exactly what type of student is being created without calling a getter/internal method to verify the student type.

Problem:

I came across this SESE page that was talking about the disadvantages of static. I became confused and wanted someone to review my code and tell me if my use of static methods are okay given what was said on that page?

The code and answer on this page provide what I need.

Answer 1

There are many code smells:

• You create a single Student class, and differentiate students by an explicit type. This is a bit sad in an object-oriented language, where one should rather attempt to use interfaces and classes to represent types, leveraging the power of abstraction and polymorphism. Typically, your design would not scale well if you were to add new student types, or specific student behaviors (SRP / OCP from SOLID are at risk).
• You create a property which should not be set not read (documents). This code is therefore a bit dubious (not useful, prone to misinterpretation).

Back to your domain: a student necessarily originates from somewhere (domestic), and may study abroad (international). In other words, I think you should simply have InternationalStudent inherit DomesticStudent, extending the class with the documents.

As for immutability, you don't have many options. The members must be final, and thus must be initialized during construction, with arguments passed to the constructor (either separately, or grouped inside a value object). Collections returned through getters must be wrapped by unmodifiable. You are already aware of this.

As for the creation method, you don't provide enough of your requirements to elicit a particular solution. Constructor calls or factories both seem fine to me.

Static methods may or may not be good, depending on the context. They do incite to consider a class as a namespace rather than a class (an object-orientation failure), and complicate testing (difficulty to mock dependencies).

Answer 2

if the constructor were made public I would then have to validate the student, using a private method, to ensure that if the student is domestic, the client can't pass a list of documents by throwing an exception. The static methods allowed the creation of the student objects and the client can't pass a List documents for domestic students, and there is no need to throw an exception.

This assumption is simply wrong because your factory method does not check any argument. So there is no difference whether you create your Student objects vis your factory method or not.

On the other hand having a factory method that really checks consistency of the parameters is a good practice. But there is no need to have that in the class itself and therefor it does not need to be static. Especially if you do not take all the parameters in one method but provide a separate setter for each. The name for is Builder Pattern.

StudenBuilder{
    private  String id;
    private  String firstname;
    private  String lastname;
    private  Collection<String> documents;
    private  StudentType typeofStudent;

   public StudenBuilder withId(String id){ 
     this.id=id;
     return this;
   }
   public StudenBuilder withFirstname(String firstname){ 
     this.firstname=firstname;
     return this;
   }
   public StudenBuilder withLastname(String lastname){ 
     this.lastname=lastname;
     return this;
   }
   public StudenBuilder with(Collection<String> documents ){
     Collections.unmodifiableList(new ArrayList<String>(documents));
      return this;
   }
   public StudenBuilder withStudentType(StudentType typeofStudent ){
      this.typeofStudent=typeofStudent;
     return this;
   }
   public Student build(){
      // check consistency
      // e.g. require documents for a certain StudentType
      return new Student(id, firstname, lastname, documents, typeofStudent);
   }
}

The usage would look like this:

Student newStudent = new StudentBuilder()
                     .withId("someId")
                     .withFirstName("some first name")
                     .withLastName("some last name")
                     .withStudentType(StudentType.SOME)
                     .build();