当重构方法时，应该选择什么方法，以便它响应干净代码的原则

Question

我有这个方法签名：

public User getActiveUser(String personId, User mainUser) throws MyExceptions {
if (personId== null) return mainUser;
User innerUser = userRepository.getByPersonId(personId);
checkForNull(innerUser);
checkIsActive(innerUser);
return innerUser;
}
private void checkForNull(User innerUser) throws UNPExceptions {
if (innerUser == null) throw new MyExceptions(USER_NOT_FOUND);
}
private void checkIsActive(User innerUser) throws UNPExceptions {
if (!innerUser.getIsActive()) throw new MyExceptions(USER_BLOCKED);
}

我把这种方法从不同的地方叫做

User user = userService.getActive(userRequest.getPersonId(), requestEntity.getUser());

我不喜欢这个代码，因为：

1( 我将2个参数传递给该方法getActiveUser(String personId, User mainUser)

如果personId为空，则mainUser总是返回。我可以在方法内部移动这个检查，但每次调用该方法之前都必须这样做。该方法在许多地方被调用。所以我把支票移到了一个地方。但它看起来很弯曲，我不知道如何绕过它。我不想传递第二个参数只是为了在方法内部进行此检查，但它看起来比在每次方法调用之前复制此检查要好。我不知道哪种解决方案更好。也许还有另一种解决方案。

2( 方法名称-getActiveUser是在撒谎。因为我在里面做更多的检查。但我不知道怎么称呼它——getActiveUserAndCheck？这也是不正确的，因为该方法负责的几个职责

3( 有必要把支票分成不同的方法吗？checkForNull(innerUser); checkIsActive(innerUser);

Answer 1

如果mainUser始终是同一个用户，则不必将其作为方法参数传递，您可以将其存储为实例字段，并在适当的时候对其进行初始化。

如果不是这样，那么可以使用AOP来处理personId为null的情况，aspect组件将负责检索mainUser。

Answer 2

使用Java8+，您可以简单地替换

checkForNull(innerUser);
checkIsActive(innerUser);
return innerUser;

通过Optional，作为：

return Optional.ofNullable(innerUser)
.filter(User::getIsActive)
.orElseThrow(() -> new MyExceptions(""));

---

如果你想默认为mainUser，你可以在这些行上做一些事情(只是这不是抛出你的自定义异常(：

return Optional.ofNullable(personId) // check for person 'null'
.map(p -> userRepository.getByPersonId(personId)) if present evaluate 'innerUser'
.filter(Objects::nonNull) // check if innerUser is null
.filter(User::getIsActive) // check if innerUser is active
.orElse(defaultUser); if any of above fails return defaultUser