空指针检查链 vs 捕获 NullPointerException

126

一个 Web 服务返回了一个巨大的 XML,我需要访问其中深度嵌套的字段。例如:

return wsObject.getFoo().getBar().getBaz().getInt()

问题在于getFoo()getBar()getBaz()可能会返回null

然而,如果我在所有情况下都检查null,代码将变得非常冗长且难以阅读。此外,我可能会错过某些字段的检查。

if (wsObject.getFoo() == null) return -1;
if (wsObject.getFoo().getBar() == null) return -1;
// maybe also do something with wsObject.getFoo().getBar()
if (wsObject.getFoo().getBar().getBaz() == null) return -1;
return wsObject.getFoo().getBar().getBaz().getInt();

写这样的东西是否可以接受?

try {
    return wsObject.getFoo().getBar().getBaz().getInt();
} catch (NullPointerException ignored) {
    return -1;
}

那样做是否被视为反模式?


30
我并不介意那么多null检查,因为wsObject.getFoo().getBar().getBaz().getInt()已经是代码异味了。请阅读"德米特法则",并根据其进行重构。然后null检查的问题也将解决。考虑使用Optional - Tom
9
使用XPath并将其留给它们进行评估怎么样? - Joop Eggen
16
这段代码很可能是由wsdl2java生成的,它不遵循"迪米特法则"。 - Adrian Cox
19个回答

152

捕获 NullPointerException 是一件非常棘手的事情,因为它们几乎可以在任何地方发生。很容易从一个 bug 中得到它并意外捕获它,然后继续仿佛一切正常,从而隐藏了一个真正的问题。处理它们非常棘手,最好完全避免。(例如,请考虑 null Integer 的自动取消装箱。)

我建议您使用Optional 类。当您想要使用存在或不存在的值时,这通常是最好的方法。

使用它,您可以像这样编写代码:

public Optional<Integer> m(Ws wsObject) {
    return Optional.ofNullable(wsObject.getFoo()) // Here you get Optional.empty() if the Foo is null
        .map(f -> f.getBar()) // Here you transform the optional or get empty if the Bar is null
        .map(b -> b.getBaz())
        .map(b -> b.getInt());
        // Add this if you want to return null instead of an empty optional if any is null
        // .orElse(null);
        // Or this if you want to throw an exception instead
        // .orElseThrow(SomeApplicationException::new);
}

为什么要使用Optional?

使用Optional代替可能为空的null值,可以使这一事实对读者非常明显清晰,并且类型系统将确保您不会意外忘记它。

您还可以方便地访问用于处理这些值的方法,例如maporElse


缺失值是有效结果还是错误?

但是还要考虑一下中间方法返回null是一个有效结果还是表示错误的迹象。如果始终是错误,则最好抛出异常而不是返回特殊值,或让中间方法本身抛出异常。


或许更多可选项?

另一方面,如果来自中间方法的缺失值是有效的,那么您也可以为它们切换到Optional吗?

然后您可以像这样使用它们:

public Optional<Integer> mo(Ws wsObject) {
    return wsObject.getFoo()
        .flatMap(f -> f.getBar())
        .flatMap(b -> b.getBaz())
        .flatMap(b -> b.getInt());        
}

为什么不选择 Optional?

我所能想到的唯一不使用 Optional 的原因是,如果这是代码中真正性能关键的部分,并且如果垃圾回收开销变成了问题。 这是因为每次执行代码时都会分配几个 Optional 对象,而虚拟机可能无法优化它们。在那种情况下,您最初的 if 测试可能更好。


8
FClass::getBar等会更短。 - Boris the Spider
1
@BoristheSpider:也许有点吧。但通常我更喜欢使用lambda表达式而不是方法引用,因为类名经常很长,而且我觉得lambda表达式更容易阅读。 - Lii
6
可以的,但请注意,方法引用可能会快一点,因为 lambda 表达式可能需要更复杂的编译时结构。Lambda 需要生成一个静态方法,这将导致非常小的性能损失。 - Boris the Spider
1
@Lii,我认为方法引用更加简洁和描述性,即使它们稍微长一些。 - shmosel
1
@likejudo:第二个示例旨在说明如果getXXX方法本身返回Optional而不是可空对象,则代码将如何显示。在这种情况下,您必须使用flatMap而不是map - Lii
显示剩余5条评论

15

我建议考虑使用Objects.requireNonNull(T obj, String message)。您可以为每个异常构建具有详细消息的链,例如:

requireNonNull(requireNonNull(requireNonNull(
    wsObject, "wsObject is null")
        .getFoo(), "getFoo() is null")
            .getBar(), "getBar() is null");

我建议您不要使用特殊的返回值,比如-1。这不符合Java的风格。Java设计了异常机制,以避免这种来自C语言的老式方式。
抛出NullPointerException也不是最佳选择。您可以提供自己的异常(将其设置为已检查以确保用户处理它,或将其设置为未检查以更轻松地处理它),或使用您正在使用的XML解析器中的特定异常。

1
Objects.requireNonNull 最终会抛出 NullPointerException 异常。因此,这与 return wsObject.getFoo().getBar().getBaz().getInt() 没有任何不同。 - Arka Ghosh
1
@ArkaGhosh,这也避免了很多像 OP 展示的 if 语句。 - Andrew Tobilko
4
这是唯一明智的解决方案。其他解决方案建议使用异常来控制流程,这是一种代码异味。顺便说一句:我认为原帖中的方法链也有异味。如果他使用三个本地变量和相应的 if 语句,情况会更清楚。此外,我认为问题比仅仅解决 NPE 更深刻:原帖作者应该问自己为什么 getter 可以返回 null。null 意味着什么?也许某些 null 对象更好?或者在一个 getter 中崩溃并抛出有意义的异常?基本上,任何事情都比用异常控制流程要好。 - Marius K.
1
使用异常来表示缺少有效返回值的无条件建议并不是很好。当方法失败且调用者难以恢复时,异常非常有用,并且最好在程序的其他部分中使用try-catch语句进行处理。如果只是简单地表示没有返回值,则最好使用Optional类,或者可能返回可空的Integer - Lii

9

假设类结构确实不受我们控制,如情况所示,我认为按照问题中建议的捕获NPE确实是一个合理的解决方案,除非性能是一个主要问题。一个小的改进可能是将抛出/捕获逻辑进行包装以避免杂乱:

static <T> T get(Supplier<T> supplier, T defaultValue) {
    try {
        return supplier.get();
    } catch (NullPointerException e) {
        return defaultValue;
    }
}

现在你可以简单地执行以下操作:
return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), -1);

return get(() -> wsObject.getFoo().getBar().getBaz().getInt(), ""); 在编译时不会出现错误,这可能会带来问题。 - Philippe Gioseffi
当然可以:https://ideone.com/X8rKiO - shmosel

5

正如评论中Tom所指出的那样,

以下语句违反了迪米特法则

wsObject.getFoo().getBar().getBaz().getInt()

你需要的是 int,可以从 Foo 中获取。 迪米特法则 建议,不要和陌生人说话。 对于你的情况,可以将实际实现隐藏在 FooBar 的后台中。

现在,您可以在 Foo 中创建方法以从 Baz 中提取 int。 最终,Foo 将具有 Bar,而在 Bar 中,我们可以访问 Int,而不直接将 Baz 暴露给 Foo。 因此,空值检查可能被分成不同的类,并且仅共享所需的属性。


4
这是值得商榷的,因为WsObject可能只是一种数据结构,是否违反了Demeter法则尚不确定。请参见此处链接:https://dev59.com/ml8e5IYBdhLWcg3wJn2l#26021695 - DerM
2
@DerM 是的,这是可能的,但既然OP已经有了解析他XML文件的东西,他也可以考虑为所需标记创建合适的模型类,以便解析库可以将它们映射。然后,这些模型类包含其自己子标记的“null”检查逻辑。 - Tom

4
您说有些方法“可能返回null”,但没有说明在什么情况下会返回null。您说捕获了NullPointerException,但没有说明为什么要捕获它。缺乏这些信息表明您对异常的作用和优势没有清晰的理解。
考虑一个类方法,该方法旨在执行某个操作,但由于其控制范围之外的情况(实际上对Java中所有方法都是如此),该方法不能保证执行该操作。我们调用该方法并返回。调用该方法的代码需要知道它是否成功。它怎么知道?它如何处理成功或失败的两种可能性?
使用异常,我们可以编写具有成功作为后置条件的方法。如果方法返回,则表示成功。如果抛出异常,则表示失败。这对于清晰度来说是一个巨大的优势。我们可以编写清晰地处理正常情况下的代码,并将所有错误处理代码移动到catch子句中。通常情况下,方法未成功的原因或方式的细节对调用者来说并不重要,因此同一catch子句可用于处理多种类型的失败。而且经常发生的情况是,一个方法根本不需要捕获异常,而只需允许它们传播到其调用者。由于程序错误引起的异常属于后一类;当存在错误时,很少有方法能够适当地做出反应。
所以,那些返回null的方法。
  • null值是否表示代码bug?如果是的话,你不应该捕获异常。而且你的代码也不应该试图猜测自己。只要写出清晰简洁的代码,并假设它能正常工作即可。一串方法调用是否清晰简洁?那就直接使用。
  • null值是否表示程序输入无效?如果是的话,抛出NullPointerException异常不是合适的,因为按照惯例,它是用来指示bug的。你可能想要抛出一个从IllegalArgumentException(如果你想要一个unchecked exception)或者IOException(如果你想要一个checked exception)派生的自定义异常。如果你的程序需要在存在无效输入时提供详细的语法错误信息,那么检查每个方法的null返回值,然后抛出适当的诊断异常是你唯一能做的事情。如果你的程序不需要提供详细的诊断信息,那么将方法调用链接在一起,捕获任何NullPointerException,然后抛出你的自定义异常是最清晰、最简洁的方式。

其中一个答案声称链式方法调用违反迪米特法则,因此是不好的。这种说法是错误的。

关于程序设计,没有绝对的规则来判断好坏。只有启发式方法:在大多数情况下是正确的(甚至几乎全部都是)。编程技能之一就是知道何时可以打破这些规则。因此,“这违反了规则X”这样简洁的断言实际上并不是一个答案。这是否是应该打破规则的情况?
“Demeter法则”实际上是关于API或类接口设计的规则。在设计类时,拥有一个抽象层次结构很有用。您可以使用低级别的类直接使用语言基元执行操作并表示高于语言基元的抽象中的对象。您可以使用中级别的类委托给低级别的类,并以比低级别的类更高的级别实现操作和表示。您可以使用高级别的类委托给中级别的类,并实现更高级别的操作和抽象。 (我在这里只谈到了三个抽象级别,但可能还有更多)。这使得您的代码可以在每个级别上使用适当的抽象表达自己,从而隐藏复杂性。 Demeter法则的理由是,如果您有一系列方法调用,那么这意味着您有一个高级别的类通过中级别的类直接处理低级别的细节,因此您的中级别类没有提供高级别类所需的中级抽象操作。但似乎这不是您在此处遇到的情况:您没有设计方法调用链中的类,它们是某些自动生成的XML序列化代码的结果(对吗?),并且调用链不会通过抽象层次结构下降,因为反序列化的XML都位于抽象层次结构的同一级别上(对吗?)。

4

我的答案与@janki几乎相同,但我想稍微修改一下代码片段如下:

if (wsObject.getFoo() != null && wsObject.getFoo().getBar() != null && wsObject.getFoo().getBar().getBaz() != null) 
   return wsObject.getFoo().getBar().getBaz().getInt();
else
   return something or throw exception;

如果有可能该对象为空,您也可以为wsObject添加一个空值检查。


3

正如其他人所说,遵守Demeter法则绝对是解决方案的一部分。另一个部分,在可能的情况下,是更改那些链接方法,使它们不能返回null。您可以通过返回空的String、空的Collection或其他虚拟对象来避免返回null,该虚拟对象意味着或执行调用者将使用null的任何操作。


3
为了提高可读性,您可能希望使用多个变量,例如:
Foo theFoo;
Bar theBar;
Baz theBaz;

theFoo = wsObject.getFoo();

if ( theFoo == null ) {
  // Exit.
}

theBar = theFoo.getBar();

if ( theBar == null ) {
  // Exit.
}

theBaz = theBar.getBaz();

if ( theBaz == null ) {
  // Exit.
}

return theBaz.getInt();

在我看来,这远不如可读性高。它会在方法中添加一堆与实际逻辑完全无关的空值检查逻辑。 - Developer102938

2

NullPointerException是一个运行时异常,因此通常不建议捕获它,而是避免它。

您将不得不在需要调用该方法的任何位置捕获异常(否则它会向上传递)。然而,如果在您的情况下,您可以继续使用那个值为-1的结果,并且您确定它不会传播,因为您没有使用可能为空的任何“片段”,那么我认为捕获它是正确的。

编辑:

我同意后来来自@xenteros的答案,最好的方法是启动自己的异常,而不是返回-1,例如您可以称之为InvalidXMLException


3
您说的“不管您是否捕获它,它都可以传播到代码的其他部分”是什么意思? - Hulk
如果在这个句子中wsObject.getFoo()出现了null,那么在代码的后面部分再次运行该查询或使用wsObject.getFoo().getBar()(例如)将再次引发NullPointerException。 - SCouto
如果我理解正确,"你必须在想要调用该方法的任何地方捕获异常(否则它将向上传播到堆栈)"这句话的措辞很不寻常。我同意这一点(并且这可能是一个问题),只是我觉得措辞有些令人困惑。 - Hulk
我会修正的,抱歉,英语不是我的母语,有时可能会出现这种情况 :) 谢谢 - SCouto

2
不要捕获NullPointerException异常。你不知道它是从哪里来的(我知道在你的情况下可能性不大,但也许还有其他东西抛出了它),而且速度很慢。 你想要访问指定的字段,为此每个其他字段都必须是非空的。这是检查每个字段的一个完美的有效理由。我可能会在一个if语句中检查它,然后创建一个可读性更好的方法。正如其他人已经指出的那样,返回-1是非常老式的,但我不知道你是否有理由这样做(例如与另一个系统交流)。
public int callService() {
    ...
    if(isValid(wsObject)){
        return wsObject.getFoo().getBar().getBaz().getInt();
    }
    return -1;
}


public boolean isValid(WsObject wsObject) {
    if(wsObject.getFoo() != null &&
        wsObject.getFoo().getBar() != null &&
        wsObject.getFoo().getBar().getBaz() != null) {
        return true;
    }
    return false;
}

编辑:是否违反Demeter法则还有争议,因为WsObject可能只是一个数据结构(请查看https://dev59.com/ml8e5IYBdhLWcg3wJn2l#26021695)。


网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接