Return 可选值，取决于流内部使用的方法抛出的异常

Question

我正在尝试实现用于处理事件的验证模块。验证模块基于简单接口：

public interface Validator {    
    Optional<ValidationException> validate(Event event);
}

我团队中的现有代码库依赖于包装异常机制——我真的无法使用它。我在实现新的验证器时遇到了问题，该验证器负责验证单个事件，有两个方面。

假设事件是 PlayWithDogEvent，它包含 Toys 一只狗可以玩。

此类事件的验证流程：

对于每个玩具，

检查它是否是一个球
如果是球，应该不会太大。

如果任何玩具不是 ball/too 大球，我的 validate(Event event) 方法应该 return Optional.of(new ValidationException("some msg")).

我已经通过以下方式实现了我的验证器：

public class ValidBallsOnlyValidator implements Validator {

    @Override
    public Optional<ValidationException> validate(Event event) {        
        try {
            event.getToys().forEach(this::validateSingleToy);
            return Optional.empty();
        } catch (InvalidToyException ex) {
            return Optional.of(new ValidationException(ex.getMessage()));
        }       
    }
    
    private void validateSingleToy(Toy toy) {
        // In real code the optional here is kinda mandatory
        Optional<Toy> potentialBall = castToyToBall(toy);
        // Im using Java 8
        if(potentiallBall.isPresent()) {
            checkIfBallIsOfValidSize(potentialBall.get(), "exampleSize");
        } else {
            throw new InvalidToyException("The toy is not a ball!")
        }
    }
    
    private void checkIfBallIsOfValidSize(Toy toy, String size) {
        if(toyTooLarge(toy, size)) throw new InvalidToyException("The ball is too big!")
    }

}

这件作品似乎工作得很好，但我对它的外观感到不舒服。我最关心的是将整个流处理放在单次尝试中是否是一个好习惯。此外，我不认为异常捕获 + returning 可选的这种混合是优雅的。

对于此类情况，我可以使用一些建议 and/or 最佳实践。

Answer 1

返回异常而不是 returning 它们很奇怪，但无论如何。（为什么不 return 一个 ValidationResult 对象呢？异常通常是用来抛出和捕获的）。

但是您可以将私有方法也更改为 return Optional 实例，这样可以更轻松地组合它们。它还可以避免混合 throwing 和 returning 和流。不确定这是否是您要找的东西？

public class ValidBallsOnlyValidator implements Validator {

    @Override
    public Optional<ValidationException> validate(Event event) 
        return event.getToys()
                .stream()
                .filter(Optional::isPresent)
                .findFirst()
                .map(ex -> new ValidationException(ex.getMessage()));
    }
    
    private Optional<InvalidToyException> validateSingleToy(Toy toy) {
        // In real code the optional here is kinda mandatory
        Optional<Toy> potentialBall = castToyToBall(toy);
        if(potentiallBall.isPresent()) {
            return checkIfBallIsOfValidSize(potentialBall.get(), "exampleSize");
        } else {
            return Optional.of(new InvalidToyException("The toy is not a ball!"));
        }
    }
    
    private Optional<InvalidToyException> checkIfBallIsOfValidSize(Toy toy, String size) {
        if(toyTooLarge(toy, size)) return Optional.of(new InvalidToyException("The ball is too big!"));
        return Optional.empty();
    }

}

Answer 2

but im uncomfortable with the way it looks.

您反对的 API 是疯狂的设计。处理傻APIs的方法大体相同：

尝试修复它'upstream'：提出拉取请求，与提出请求的团队交谈等。
当且仅当该选项已用尽时，然后 [A] 编写您必须编写的任何丑陋的 hacker，以使其工作，[B] 将丑陋限制在尽可能小的代码片段中；这可能涉及编写一个 'contains' 丑陋的包装器，最后 [C] 不用担心限制 'ugly is okay here' 区域内的代码优雅。

API 之所以奇怪，是因为它既在验证错误，又没有利用他们的错误所带来的好处（例如，如果我认为他们的方法是错误的，那么在至少他们没有在他们的方法上做得最好。

具体来说，异常是一个 return 值，从某种意义上说，它是从方法 return 的一种方式。为什么不是那个界面：

public interface Validator {    
    void validate(Event event) throws ValidationException;
}

更一般地说，验证不是 'there is at most one thing wrong' 情况，这会导致 'it feels weird to write a try/catch around the whole thing' 的问题。

很多事情都可能是错误的。可能有 5 个玩具，其中一个是球，但太大了，另一个是吱吱作响的玩具。只报告一个错误（大概是任意选择的错误）很奇怪。

如果你打算采用不抛出验证异常但returning验证问题的方式，那么这些问题应该不是首先，但是其他一些对象，并且，您应该使用 List<ValidationIssue> 而 而不是 和 Optional<ValidationIssue>。你已经摆脱了一个可选的，这总是一个胜利，你现在可以一次处理多个问题。如果处理所有这些的 'end point' 当时根本无法处理多个问题，那没关系：他们可以将该列表视为有效的可选列表，而 list.isEmpty() 作为 'all is well' 指标，以及 list.get(0) 否则用于解决第一个问题（这是这个一次一个错误的系统可以处理的唯一问题）。

这涉及到代码优雅，这是定义该词的唯一有意义的方式 'elegance'：代码更易于测试、更易于理解且更灵活。它更灵活：如果以后处理验证错误的端点代码更新为能够处理多个错误，您现在可以在不触及生成验证问题对象的代码的情况下执行此操作。

于是，全部重写。或者：

进行 API 设计，重点是 THROW 异常，而不是将其推入可选的，-or-
使 API 基于列表，同时摆脱可选（耶！）并且可能不适用于 extends SomeException 的验证问题对象。如果你不想扔它，就不要把它变成可扔的。

如果那不行，大多数情况下只是不要太担心优雅 - 一旦你被迫使用糟糕的设计，优雅就远离 table APIs.

然而，几乎总是有一些样式注释可以提供给任何代码。

return Optional.of(new ValidationException(ex.getMessage()));

通常，这是非常糟糕的异常处理，您的 linter 工具应该将其标记为 unacceptable。如果包装异常，您希望保留原因以保留堆栈跟踪和任何特定于异常类型的信息。您通过忽略关于 ex 的所有内容来摆脱所有这些，除了它的消息。通常，这应该是 new ValidationException("Some string that adds appropriate context", ex) - 从而保留链。如果没有要添加的上下文/很难想象这可能是什么，那么您根本不应该包装，而是继续抛出原始异常。

然而，考虑到异常在这里被滥用，也许这段代码是好的——这又回到了中心点：一旦你致力于使用设计糟糕的 API，经验法则正确的代码风格直接出现 window.

private void checkIfBallIsOfValidSize(Toy toy, String size) {
       if(toyTooLarge(toy, size)) throw new InvalidToyException("The ball is too big!")
}

是的，这是个好主意——虽然 API 希望您不要抛出异常，而是将它们包装在可选值中，但这部分很糟糕，您通常不应该使错误永久化，即使这意味着您的代码开始出现不同的风格。

event.getToys().forEach(this::validateSingleToy);

一般来说，直接使用forEach方法，或者.stream().forEach()，都是code smell。 forEach 只应在两种情况下使用：

它是一堆流操作的终端（.stream().filter().flatMap().map()....forEach - 没问题）。
您已经有一个 Consumer<T> 对象，并希望它为列表中的每个元素运行。

你两者都没有。这段代码最好写成：

for (var toy : event.getToys()) validateSingleToy(toy);

Lambdas 有 3 个缺点（如果按预期使用 lambdas，即在某些不同的上下文中可能运行的代码，这些缺点就会变成优点）：

控制流不透明。
不是 mutable 局部变量透明。
未检查异常类型透明。

你失去了 3 样东西，而你在 return 中一无所获。当有 2 种同样简洁明了的方法来做同一件事，但两者中的一种适用于严格的超集场景时，总是以超集样式编写它，因为代码一致性是一个有价值的目标，这会带来更高的一致性（这是值得的，因为它减少了风格摩擦并降低了学习曲线）。

这条规则适用于此。

Return 可选值，取决于流内部使用的方法抛出的异常

Return optional value, depending on exception thrown by method used inside stream

java

exception

code-cleanup