如何使用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
的话。但是,我会担心在一个月内重新访问此代码。您可能需要斜视一下,然后才能说服它完成了您打算做的事情。最后,一些一般风格的评论。
NullPointerException
。对于这些示例,我坚持使用IllegalArgumentException
,因为这就是OP所使用的。 List<Integer>
代替ArrayList<Integer>
作为参数类型以及可能的字段类型。这样可以在适当的情况下(例如,使用JDK 9的List.of
)使用不可修改的列表。