Using static factory methods to prevent unwanted input

Question

Many years ago, I posted a problem I had with some code and received a well written detailed answer that suggested the use of factory methods. I liked this approach, because I can provide a method name to let clients know what was being created, and to prevent wrong parameters.

// Left out additonal parameters and validation
private Student(StudentType type, List<String> documents) {
    this.type = type;
    this.documents = documents;
}

public static Student createDomestic() {
    return new Student(StudentType.DOMESTIC), Collections.emptyList());
}

public static Student createInternational(List<Document> documents) {
    return new Student(StudentType.INTERNATIONAL, new ArrayList<>(documents);
}

Benefits:

• I don't need a private method to validate the List based on the StudentType, it made the code shorter and cleaner. All I needed was to check if the List provided for the International students contained documents (by checking the length of the List).
• The static factory prevents clients from assigning a Domestic student a List of documents, which according to the requirements it shouldn't have.

Leaving aside the POLA, is using a static factory method the way it was suggested on SO, considered a code smell or bad practice?

Answer 1

This seems like a perfectly good use of static factory methods.

The requirement that a international student should have documents, but a domestic student shouldn't, does make me question why you didn't use an object hierarchy in which you have DomesticStudent and InternationalStudent classes deriving from Student, but I'll assume that there were other requirements driving that decision.

Answer 2

An simpler alternative might be to have a single constructor taking a single parameter, the List. Skeletal code:

public Student(List docs) {
  this.documents = docs;
  this.type = (docs.length() > 0)?
                         StudentType.INTERNATIONAL :
                         StudentType.DOMESTIC;
}

Not sure if this makes more sense for your application.

Also, makes you wonder if the type field is required - couldn't you just check for docs.length? e.g.

public boolean isInternational() {
  return docs.length() > 0;
}