验证由另一个参数确定的构造函数参数

Question

我有以下enum

public enum StudentType {
    Domestic, International;
}

还有一个具有以下构造函数的 Student 类：

//Left out additonal parameters and validation
public Student(StudentType type, List<String> documents){

  if(type == null){
     throw new IllegalArgumentException("You must provide Student type");
  }
  this.type = type;
  this.documents = this.validateList(documents);
}

还有一个验证列表的私有方法：

private List<String> validateList(List<String> validate){
   if(this.type == StudentType.Domestic && validate.isEmpty()){
        return validate;
    }
    else 
      if(this.type == StudentType.Domestic && !validate.isEmpty()){
        return Collections.emptyList();
   }
   return new ArrayList<String>(validate);
}

我的计划是让 Student 类不可变。

说明：

只有国际学生需要文件（护照等）。国内学生不需要任何文件。

在我的constructor 中，我检查以确保类型不是null，客户必须提供国内或国际。

在我的私人 validateList method 中，我检查学生类型，以及 List 是否为空。如果是国内且为空，则返回列表，如果是domensitc且不为空，则返回空集合，否则返回文档的ArrayList<String>。

我的问题是检查私有方法中的类型是否是代码异味？我担心的是一个参数（文件列表）由学生类型确定/验证。如果检查私有方法中的类型是代码异味，我应该怎么做？

Answer 1

我发现您发布的代码存在一些问题，但不是您认为的问题。

首先，我们不知道documents 来自哪里。我认为这只是代码中的一个错误，实际上它是第二个参数。

现在，代码：

private List<String> validateList(List<String> validate){
   if(this.type == StudentType.Domestic && validate.isEmpty()){
        return validate;
    }
    else 
      if(this.type == StudentType.Domestic && !validate.isEmpty()){
        return Collections.emptyList();
   }
   return new ArrayList<String>(validate);
}

首先，如果参数无效（就像您对 null 学生类型所做的那样），而不是拒绝参数，您只需忽略参数并使用空列表代替。一般来说，这不是一个好主意。如果调用者传递了一个非空列表，它当然不希望这个列表被默默地忽略。如果列表应该为空，则通过抛出异常来拒绝非空列表。

第二：最后一行代码表明您想要制作作为参数传递的列表的防御性副本。但是您不会在 if 块的第一个分支中这样做。并且由于这两个分支无论如何都包含存储一个空集合，因此可以将其替换为

private List<String> validateList(List<String> validate) {
    return this.type == StudentType.Domestic ? Collections.emptyList() : new ArrayList<>(validate);
}

最后，（回到我的第一点），既然如果学生类型是国内的话，调用者永远不应该传递一个非空列表，你可以通过使用两个工厂方法而不是构造函数来实现这一点：

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

Answer 2

我会用以下方式重写validateList 方法：

private List<String> validateList(List<String> validate){
   if(this.type == StudentType.Domestic){
        return Collections.emptyList();
   }
   return new ArrayList<String>(validate);
}

说明

如果学生类型是国内的，只要您只需要不返回任何文件，我将两个条件合二为一。国内学生有没有证件对我们来说并不重要，我们不需要检查。