25

我和我的同事就这段代码进行了辩论： 

var y = null;
if (x.parent != null)
    y = x.parent.somefield;

我的观点是，在代码所在的地方，x.parent不应该为空。当它为空时，我们有一个严重的问题，我想知道它！因此，不应存在空值检查并让下游异常发生。

我的同事说这是防御性编程。空值检查确保代码不会破坏应用程序。

我的问题是，这是防御性编程吗？还是不好的做法？

注意：重点不是谁对。我试图从这个例子中学习。

4

5 回答 5

17

有趣的问题。从我的角度来看，是否包括检查取决于数据的验证程度，数据来自哪里以及检查失败时会发生什么。

“x.parent 不应该为空”是一个严肃的假设。你需要非常确定它才能安全，当然有经典的“它永远不会发生”......直到它发生，这就是为什么我认为回顾可能性很有趣。

我看到两件不同的事情需要考虑。

数据从何而来？

  • 如果它来自同一个类中的另一个方法，或者来自某个相关类，因为你或多或少地完全控制它，放松你的防御是合乎逻辑的，因为你可以合理地假设它不太可能有坏数据开始有，或者如果发生这种情况，在调试/测试期间及早发现错误，甚至为它进行一些单元测试是相当容易的。

  • 相反的情况是，如果它是用户输入的数据，或者从文件中读取的数据，或者从 URL 中读取的数据，一般来说是外部的任何东西。由于您无法控制程序正在处理的内容：无论如何，在以任何方式使用它之前尽可能彻底地验证它，因为您会接触到可能导致问题的虚假/缺失/不完整/不正确/恶意信息路径。

  • 中间情况可能是输入来自同一系统内的另一层/层。决定是否进行全面验证或认为它是理所当然的更加困难，因为它是另一个内部软件，但以后可能会被替换或独立修改。我更倾向于在跨越边界时再次验证。

验证怎么办？

  • 在某些情况下，使用if（如您的示例）简单地跳过某些分配或使用可能很好。例如，如果用户正在输入一些数据，而这只是显示工具提示或其他次要信息，则跳过可能是安全的。但是，如果这段代码做了一些重要的事情，进而满足了一些强制性条件或执行了一些其他过程，那么这不是正确的方法，因为它会导致下一次代码运行出现问题。问题是，当你跳过一些代码时，这样做必须是安全的，没有任何副作用或不良后果，否则你会隐藏一些错误，并且在以后的开发阶段很难调试。

  • 优雅地中止当前过程是早期验证的一个不错的选择，当失败完全在意料之中并且您确切地知道如何响应它时。一个示例可能是缺少必填字段，该过程被中断，并向用户显示一条消息，询问缺少的信息。简单地忽略错误是不行的，而且还不够严重到抛出破坏正常程序流程的异常。当然，您仍然可以使用异常，具体取决于您的架构，但无论如何都要捕获它并优雅地恢复。

  • 当“不可能”真正发生时，抛出异常始终是一种选择。在这些情况下，您可能无法为继续进行某些变化或仅取消当前流程提供合理的响应，这可能是由于某处的错误或输入错误，但重要的是您想知道它并拥有关于它的所有细节，所以最好的方法是让它尽可能大声地爆炸，以便异常冒泡并到达一个中断所有内容的全局处理程序，保存到日志文件/DB/whatever，发送一个向您报告崩溃并找到恢复执行的方法（如果可行或安全）。至少如果您的应用程序崩溃，请以最优雅的方式这样做，并留下痕迹以供进一步分析。

与往常一样，这取决于情况。但是仅仅使用 if 来避免编写异常处理程序肯定是一种不好的做法。它必须始终存在，然后一些代码可能会依赖它——无论是否合适——如果失败不是关键的话。

于 2014-03-07T01:35:16.740 回答
13

看起来您的同事误解了“防御性编程”和/或例外情况。

防御性编程

防御性编程是关于防止某些类型的错误。

在这种情况下x.parent == null是一个错误，因为您的方法需要使用x.parent.SomeField. 如果parent为空，那么 的值SomeField显然是无效的。使用无效值执行的任何计算或任务都会产生错误和不可预测的结果。

所以你需要防范这种可能性。一个很好的保护方法是NullPointerException如果你发现它就扔一个x.parent == null。该异常将阻止您从SomeField. 它会阻止您使用无效值进行任何计算或执行任何任务。它将中止所有当前工作，直到错误得到适当解决。

注意异常不是错误；一个无效值parent是实际错误。异常实际上是一种保护机制。异常是一种防御性编程技术，它们是不可避免的。

由于 C# 已经抛出异常，因此您实际上不必执行任何操作。事实上，您的同事“以防御性编程的名义”所做的努力实际上是在撤消该语言提供的内置防御性编程。

例外

我注意到许多程序员对异常过度偏执。异常本身不是错误，它只是报告错误。

您的同事说：“空检查确保代码不会破坏应用程序”。这表明他认为异常会破坏应用程序。它们通常不会“破坏”整个应用程序。

如果糟糕的异常处理使应用程序处于不一致的状态，异常可能会破坏应用程序。（但如果错误被隐藏，这种情况更有可能发生。）如果异常“逃脱”线程，它们也可能破坏应用程序。（转义主线程显然意味着您的程序已经相当不优雅地终止了。但即使转义子线程也已经够糟糕了，操作系统的最佳选择是 GPF 应用程序。）

然而，异常将中断（中止）当前操作。这是他们必须做的事情。因为如果您编写一个名为DoSomethingwhich calls的方法DoStep1；错误意味着无法正常工作。_ 继续打电话是没有意义的。DoStep1DoSomethingDoStep2

但是，如果在某些时候您可以完全解决特定错误，那么请务必：这样做。但请注意强调“完全解决”；这并不意味着只是隐藏错误。此外，仅记录错误通常不足以解决它。这意味着要达到这样的程度：如果另一个方法调用您的方法并正确使用它，“已解决的错误”不会对调用者正确完成其工作的能力产生负面影响。（不管来电者是什么。）

完全解决错误的最佳示例可能是在应用程序的主处理循环中。它的工作是：等待队列中的一条消息，从队列中拉出下一条消息并调用适当的代码来处理该消息。如果在返回主消息循环之前引发了异常并且未解决，则需要解决它。否则异常将逃逸主线程，应用程序将被终止。

许多语言在其标准框架中提供了默认的异常处理程序（具有程序员覆盖/拦截它的机制）。默认处理程序通常只会向用户显示错误消息，然后吞下异常。

为什么？因为如果您没有实现糟糕的异常处理，您的程序将处于一致状态。当前消息已中止，可以处理下一条消息，就好像没有任何问题一样。您当然可以覆盖此处理程序以：

  • 写入日志文件。
  • 发送调用堆栈信息以进行故障排除。
  • 忽略某些类别的异常。（例如Abort，可能意味着您甚至不需要告诉用户，可能是因为您之前显示了一条消息。）
  • 等等

异常处理

如果您可以在不首先引发异常的情况下解决错误，那么这样做会更干净。但是，有时错误无法在首次出现的地方解决，或者无法提前检测到。在这些情况下，应该引发/抛出异常以报告错误，并通过实现异常处理程序（C# 中的catch块）来解决它。

注意：异常处理程序有两个不同的目的：首先，它们为您提供一个执行清理（或回滚代码）的地方，特别是因为存在错误/异常。其次，它们提供了一个解决错误和吞并异常的地方。注意：在前一种情况下，重新引发/抛出异常非常重要，因为它尚未解决。

在关于抛出异常和处理它的评论中，你说：“我想这样做，但我被告知它会在任何地方创建异常处理代码。”

这是另一个误解。根据前面的旁注，您只需要在以下情况下进行异常处理：

  • 您可以解决错误，在这种情况下您就完成了。
  • 或者你需要实现回滚代码的地方。

这种担忧可能是由于有缺陷的因果分析造成的。您不需要因为抛出异常而回滚代码。引发异常的原因还有很多。回滚代码是必需的，因为如果发生错误，该方法需要执行清理。换句话说，在任何情况下都需要异常处理代码。这表明针对过度异常处理的最佳防御方法是以减少错误清理需求的方式进行设计。

所以不要“不抛出异常”，避免过多的异常处理。我同意过多的异常处理是不好的（参见上面的设计考虑）。但是当你应该回滚的时候不回滚要糟糕得多，因为你甚至不知道有一个错误。

于 2014-06-16T13:43:29.557 回答
2

我根本不会称之为防御性编程——我会称之为“啦啦啦我听不见你”的编程:) 因为代码似乎有效地忽略了潜在的错误条件。

显然，我们不知道您的代码接下来会发生什么，但由于您没有包含else子句，我只能假设您的代码即使在实际x.parent 是null.

请记住，“不应该为空”和“绝对、肯定地保证永远不会为空”不一定是同一回事；因此，在这种情况下，当您尝试取消引用y.

那么问题是 - 在您尝试解决的问题（“域”）的上下文中，什么是更可接受的，这取决于您y以后打算做什么。

  • 如果y在null此代码之后没有问题（假设您稍后对 进行防御性检查y!=null）那么没关系 - 尽管我个人不喜欢这种风格 - 你最终会防御性地检查每一个取消引用，因为你永远不可能很确定你是否是一个远离崩溃的空引用......

  • 如果因为稍后会导致异常或丢失数据y而不能在代码之后，那么当您知道不变量不正确时继续下去就是一个坏主意。null

于 2014-03-07T01:06:09.327 回答
1

简而言之，我想说这不是防御性编程。我同意那些认为此代码隐藏系统错误而不是暴露和修复的人的观点。此代码违反了“快速失败”原则。

当且仅当 x.parent 是一个强制的非空属性（从上下文中似乎很明显）时，这是正确的。但是，如果 x.parent 是一个可选属性（即可以合理地具有空值），那么这段代码可以根据您表达的业务逻辑来确定。

我一直在考虑使用空值（0、“”、空对象）而不是需要大量无关 if 语句的空值。

于 2014-03-07T22:21:43.747 回答
-2

经过几年对这个问题的思考，我使用了以下“防御性”编程风格——我的 95% 的方法返回字符串作为成功/失败的结果。

我返回 string.Empty 表示成功，如果失败则返回信息文本字符串。在返回错误文本的同时，我将其写入日志。

public string Divide(int numA, int numB, out double division)
{
    division = 0;
    if (numB == 0)
        return Log.Write(Level.Error, "Divide - numB-[{0}] not valid.", numB);

    division = numA / numB;
    return string.Empty;
}

然后我使用它：

private string Process(int numA, int numB)
{
    double division = 0;
    string result = string.Empty;
    if (!string.IsNullOrEmpty(result = Divide(numA, numB, out divide))
        return result;

    return SendResult(division);
}

当您拥有日志监控系统时，它会让系统继续显示，但会通知管理人员。

于 2017-06-30T17:23:41.507 回答