如何使用Optionals将此功能重写为更多Java 8?还是应该保留原样?

public void setMemory(ArrayList<Integer> memory) {
    if (memory == null)
        throw new IllegalArgumentException("ERROR: memory object can't be null.");
    if (memory.contains(null))
        throw new IllegalArgumentException("ERROR: memory object can't contain null value.");

    this.memory = memory;
}

最佳答案

在这种情况下,我通常会避免使用Optional,因为它会掩盖正在发生的事情。

但是首先我想提一下,原始代码使调用者可以保留对包含类的内部字段memory的引用。也许您相信 call 者不是恶意的,但是 call 者可能不小心重用了作为参数传递的列表。如果这样做,尽管进行了细致的参数检查,但memory列表最终可能最终包含空值。或者,它可能会意外更改,从而导致其他故障。

解决方案是对参数列表进行防御性复制。这样做的简单方法如下:

public void setMemory(ArrayList<Integer> memory) {
    if (memory == null)
        throw new IllegalArgumentException("memory is null");

    List<Integer> temp = new ArrayList<>(memory);

    if (temp.contains(null))
        throw new IllegalArgumentException("memory contains null");

    this.memory = temp;
}

请注意,在检查之前,已创建副本并将其存储在本地变量temp中。显然,您不想在检查列表是否包含空值之前将其存储到该字段中。但是检查是否包含空值应该在副本上完成,而不是在参数列表上完成,否则,调用方可以在检查之后但在副本之前修改列表。 (是的,这很偏执。)

如果您不关心确切的异常消息,则可以将其缩短如下:
public void setMemory(ArrayList<Integer> memory) {
    List<Integer> temp;
    if (memory == null || ((temp = new ArrayList<>(memory)).contains(null)))
        throw new IllegalArgumentException("memory is or contains null");
    this.memory = temp;
}

现在可以将其重写为使用Optional:
public void setMemory(ArrayList<Integer> memory) {
    this.memory = Optional.ofNullable(memory)
                          .map(ArrayList::new)
                          .filter(list -> ! list.contains(null))
                          .orElseThrow(() -> new IllegalArgumentException("memory is or contains null"));
}

与我经常看到的Optional的常见滥用:-)相比,这还算不错。这里的链接可避免创建局部变量,这是一个胜利。逻辑非常简单,特别是如果前脑上有Optional的话。但是,我会担心在一个月内重新访问此代码。您可能需要斜视一下,然后才能说服它完成了您打算做的事情。

最后,一些一般风格的评论。
  • 通常(至少在JDK中)首选在这些情况下使用NullPointerException。对于这些示例,我坚持使用IllegalArgumentException,因为这就是OP所使用的。
  • 我建议您使用List<Integer>代替ArrayList<Integer>作为参数类型以及可能的字段类型。这样可以在适当的情况下(例如,使用JDK 9的List.of)使用不可修改的列表。
  • 07-24 09:36
    查看更多