重构方法时选择什么方法，使其符合干净代码的原则？

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)

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

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

3) 是否有必要将检查分成不同的方法？ checkForNull(innerUser); checkIsActive(innerUser);

Answer 1

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

如果不是这样，您可以使用 AOP 来处理具有 null personId 的情况，并且方面组件将负责检索 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