我该如何进行防御性编程?

38
我正在使用一个小程序来创建数据库连接:

之前

public DbConnection GetConnection(String connectionName)
{
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
   DbConnection conn = factory.CreateConnection();
   conn.ConnectionString = cs.ConnectionString;
   conn.Open();

   return conn;
}

然后我开始查看.NET框架文档,看看各种事物的记录行为是什么,并尝试处理它们。
例如:
ConfigurationManager.ConnectionStrings...

文档说明称,如果无法检索到集合,则调用 ConnectionStrings 会抛出 ConfigurationErrorException。在这种情况下,我无法处理此异常,因此我将让它发生。
下一部分是实际索引ConnectionStrings以查找connectionName
...ConnectionStrings[connectionName];

在这种情况下,ConnectionStrings文档中说,如果找不到连接名称,则该属性将返回null。我可以检查是否发生了这种情况,并抛出异常,让高层知道他们提供了无效的connectionName。
ConnectionStringSettings cs= 
      ConfigurationManager.ConnectionStrings[connectionName];
if (cs == null)
   throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

我重复相同的练习,使用:
DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);

GetFactory”方法没有记录,如果找不到指定“ProviderName”的工厂会发生什么。虽然它没有记录返回null,但我仍然可以防御性地进行检查以获取null:

DbProviderFactory factory = 
      DbProviderFactories.GetFactory(cs.ProviderName);
if (factory == null) 
   throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

接下来是构建DbConnection对象:
DbConnection conn = factory.CreateConnection()

再次,文档没有说明如果无法创建连接会发生什么,但我可以检查返回的对象是否为空值:
DbConnection conn = factory.CreateConnection()
if (conn == null) 
   throw new Exception.Create("Connection factory did not return a connection object");

接下来是设置Connection对象属性:
conn.ConnectionString = cs.ConnectionString;

这段话的意思是:文档没有说明如果无法设置连接字符串会发生什么。会抛出异常吗?还是忽略它?像大多数异常一样,如果在尝试设置连接的 ConnectionString 时出现错误,我无法恢复。所以我什么也不做。
最后,打开数据库连接:
conn.Open();

DbConnection的Open方法是抽象的,因此取决于从DbConnection派生的任何提供程序来决定它们抛出什么异常。在抽象的Open方法文档中也没有关于如果发生错误会发生什么的指导。如果连接时出现错误,我知道我无法处理它 - 我必须让它上升到调用方,让其向用户显示一些UI,并让他们重试。

之后

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
    if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");

   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

摘要

所以我的四行代码函数变成了12行,并需要查找5分钟的文档。最终,我确实发现了一种情况,即某个方法允许返回null值。但在实践中,我只是将访问违规异常(如果尝试调用null引用上的方法)转换为InvalidArgumentException

我还捕获了两种可能存在null返回对象的情况;但同样,我只是将一个异常替换为另一个异常。

积极的一面是,它确实发现了两个问题,并解释了异常消息中发生了什么,而不是在后面出现问题(即责任到此为止)。

但这值得吗?这是否过度防御式编程?


2
这很不错。我唯一会做不同的事情是使用String.Format来构建消息,而不是附加字符串。我还会将消息放入资源中,以便进行本地化。 - John Saunders
3
哪种程序设计更易于调试?哪种更易于维护?增加的复杂性是否值得为其付出努力? - Nosredna
3
最简单的代码是没有任何错误检查的代码。而且没有任何形式的错误检查也更易于阅读。 - Ian Boyd
14个回答

31

手动检查配置并抛出异常并没有比让框架在缺少配置时抛出异常更好。你只是重复了框架方法内部已经进行的前置检查,这会使你的代码冗长而且没有益处。(事实上,通过将所有内容都作为基本的异常类抛出,你可能会删除信息。框架抛出的异常通常更具体。)

编辑:这个答案似乎有点有争议,因此稍微解释一下:防御性编程意味着“为意外情况做好准备”(或者“做好偏执狂的心理准备”),其中一种方法是进行大量的前置检查。在许多情况下,这是一个好的惯例,但是像所有惯例一样,成本应该与好处相比较。

例如,抛出“无法从工厂获取连接”异常没有任何好处,因为它并没有说明为什么无法获取提供程序 - 如果提供程序为空,下一行将立即抛出异常。因此,前置检查的成本(开发时间和代码复杂性)是不合理的。

另一方面,验证连接字符串配置是否存在的检查可能是合理的,因为异常可以帮助开发人员解决问题。你在下一行将得到null异常,但是它并没有说明缺少的连接字符串的名称,因此,前置检查确实提供了一些价值。如果你的代码是组件的一部分,那么价值非常大,因为组件的用户可能不知道组件需要哪些配置。

防御性编程的另一种解释是,你不仅应该检测错误条件,还应该尝试从可能发生的任何错误或异常中恢复过来。我认为这通常不是一个好主意。

基本上,你只应该处理你可以采取行动的异常。无论如何无法恢复的异常应该被传递给顶层处理程序。在Web应用程序中,顶层处理程序可能只显示一个通用错误页面。但是,在大多数情况下,如果数据库离线或某些重要的配置丢失,真的没有太多可以做的。

在某些情况下,这种防御性编程是有意义的,特别是当您接受用户输入且该输入可能导致错误时。例如,如果用户提供一个URL作为输入,并且应用程序尝试从该URL获取某些内容,那么非常重要的是您检查URL是否正确,并处理可能由请求引起的任何异常。这样可以向用户提供有价值的反馈。


3
我不同意。对于一段代码来说,捕获一般异常并将其更改为更具体的消息或检查前置条件是有意义的。是的,他本可以允许NullReferenceException继续传播,但那样做并不能提示调用者异常是由于无法构建连接而引起的。 - John Saunders
我很惊讶这是最高评分的答案。我同意John Saunders的观点。 - CiscoIPPhone
3
我不同意John的看法。事实上,通过异常包含的堆栈跟踪,调用者确实可以推断出引发异常的原因。 - Jacob
1
我同意Jacob的看法(保持链表运行)。明智的做法是有一个AppDomain.UnexpectedException处理程序,将exception.InnerException链和所有堆栈跟踪转储到某种日志文件中,然后调用Environment.FastFail。 - Daniel Earwicker
3
没错。对于异常情况的经验法则是,如果发生异常是由于配置或编码错误引起的,则不要捕获它。如果偶尔预计会在用户输入过程中遇到某些异常情况,则可以继续捕获它。上面展示的“防御性编码”例子确实是一种“编码恐怖”。 - Dave Markle
显示剩余2条评论

13

嗯,这取决于你的受众是谁。

如果你正在编写希望许多其他人使用的库代码,并且不会与你讨论如何使用它,则这并不过度。他们会欣赏你的努力。

(话虽如此,如果你这样做,我建议你定义比仅使用System.Exception更好的异常,以使想要捕获其中一些异常但不想捕获其他异常的人更容易。)

但是,如果你只是自己使用它(或你和你的朋友),那么显然这就过度了,并且可能通过使你的代码不够可读来最终损害你。


7
我希望我的团队能像这样编写代码。大多数人甚至没有理解防御式编程的重点。最好的做法是将整个方法包装在try catch语句中,并让所有异常都由通用异常块处理!
向你致敬,Ian。我理解你的困境。我自己也经历过同样的事情。但你所做的可能会帮助某些开发人员节省几个小时的键盘敲击。
记住,在使用.net框架API时,你期望得到什么?什么看起来自然?在你的代码中做同样的事情。
我知道这需要时间。但是,质量需要花费代价。
PS:你真的不必处理所有错误并抛出自定义异常。请记住,你的方法只会被其他开发人员使用。他们应该能够自己找出常见的框架异常。那不值得麻烦。

1
好吧,至少在异常环境中,默认的失败状态是“停止”。如果你正在使用像C这样的东西,并且你的团队忽略了错误,你的应用程序将进入一些未定义的状态,并在未来的某个不确定点死亡。 - Bernard
我同意Bernard的观点。我具体是在谈论C#方面的。 - bobbyalex
我知道。应该是“至少你是”,抱歉。 - Bernard
我不明白为什么要加那么多try/catch语句来避免“键盘狂敲”的情况。堆栈跟踪可以显示异常的起源,解决问题也变得微不足道。配置和编码错误不应该被捕获 - 让框架自己处理。虽然我认为防御性编程很重要,但它是有上下文的,这里并没有增加任何价值。 - Cory House
Cory,我完全同意你的观点,我的意思也只是这样。你不必处理所有的异常,只需要处理与当前方法相关的有意义的异常即可。我的观点是,你不应该将所有方法都包装在try catch中,并使方法吞噬自己的异常,有些异常需要传递给调用者。 - bobbyalex

6

你的“before”示例具有清晰简洁的特点。

如果出现问题,框架最终会抛出异常。如果您无法处理异常,则最好让它沿着调用栈向上传播。

然而,在框架内部深处抛出异常时,真正的问题可能并不明显。如果您的问题是没有有效的连接字符串,但框架抛出类似于“无效使用空值”的异常,那么有时最好捕获异常并重新抛出具有更有意义的消息。

我经常检查空对象,因为我需要一个实际的对象来操作,如果对象为空,则抛出的异常至少是含糊不清的。但是,只有在我知道会发生这种情况时才检查null对象。一些对象工厂不返回null对象;它们会抛出异常,这种情况下检查null将是无用的。


我最初在代码中加入了注释,以逻辑方式描述正在发生的事情。但为了让这个SO问题看起来精简明了,我把它们删掉了,没有注释 :) - Ian Boyd
3
对于理解对象及其用途的人来说,未加注释的代码相当明显。 - Robert Harvey
注释的目的是帮助下一个人理解对象及其用途 - 而很可能下一个人就是我,8个月后。在第一行之前加上类似于“静态 ConfigurationManager 类从 web.config 中提取连接字符串信息”的注释真的有助于解释正在发生的事情。“哦,原来它们存储在 web.config 中?哦,没错。” - Ian Boyd
我不同意Ian的观点。程序员不应该通过注释来记录框架 - 这是框架文档的职责。注释应该解释为什么要这样做,而代码则说明在做什么。注释不应该成为技术低下的补救措施。 - Cory House

3

我认为我不会编写任何空引用检查逻辑 - 至少不是你所做的方式。

从应用程序配置文件中获取配置设置的程序在启动时都会检查所有这些设置。我通常构建一个静态类来包含这些设置,并在应用程序的其他地方引用该类的属性(而不是ConfigurationManager)。这样做有两个原因。

首先,如果应用程序没有正确配置,它将无法工作。我宁愿在程序读取配置文件的时候就知道这一点,而不是在将来某个时刻尝试创建数据库连接时才知道。

其次,检查配置的有效性实际上不应该是依赖于配置的对象的关注点。如果您已经在前面执行了这些检查,那么在代码中到处插入检查也没有意义。(当然,也有例外情况 - 比如长时间运行的应用程序,在这种程序中,您需要能够在程序运行时更改配置,并使这些更改反映在程序的行为中;在这种程序中,您需要在需要设置时使用ConfigurationManager。)

我也不会在GetFactoryCreateConnection调用上进行空引用检查。您如何编写测试用例来测试该代码?您无法这样做,因为您不知道如何使这些方法返回null - 您甚至不知道使这些方法返回null是可能的。因此,您已经用另一个更重要的问题替换了一个问题 - 在那些同样神秘的条件下,您的程序将运行您没有测试过的代码,并且可能会抛出NullReferenceException异常。


1

你的方法文档缺失了. ;-)

每个方法都有一些定义好的输入和输出参数以及一个定义好的结果行为。在你的情况下,可以这样做:"在成功的情况下返回有效的打开连接,否则返回null(或者抛出一个XXXException,根据你的喜好)。记住这个行为,然后你可以决定你应该以多么防御性的方式编程。

  • 如果你的方法应该提供关于失败的详细信息,那就像你之前做的那样,检查并捕获所有可能的错误,并返回相应的信息。

  • 但是,如果你只对一个打开的DBConnection感兴趣或者只希望在失败时返回null(或其他自定义异常),那么只需将所有内容放在try/catch块中,在错误时返回null(或其他异常),在正常情况下返回对象。

所以我想说,这取决于方法的行为和预期的输出。


你会抛出异常而不是返回异常,对吧? - Svish
当然,异常应该总是被抛出(从未返回过一个)。 - MicSim

1
通常情况下,应该捕获特定于数据库的异常,并将其重新抛出为更一般的异常,例如(假设)DataAccessFailure 异常。在大多数情况下,更高级别的代码不需要知道您正在从数据库中读取数据。
快速捕获这些错误的另一个原因是它们通常在其消息中包含一些数据库详细信息,例如“找不到表:ACCOUNTS_BLOCKED”或“用户键无效:234234”。如果这些错误传播到最终用户,会出现以下几种问题:
  1. 令人困惑。
  2. 潜在的安全漏洞。
  3. 对公司形象尴尬(想象一下客户阅读带有粗糙语法的错误消息)。

1
你不应该从异常中删除有价值的信息!相反,你应该确保顶层处理程序不会向最终用户显示实际的异常和堆栈跟踪。 - JacquesB
我并不是说这个应该被移除,但是它必须被处理。而且你也不想在顶层代码中捕获 SQLException。 - quant_dev

1

我的经验法则是:

如果抛出异常的消息与调用者相关,则不要捕获异常。

因此,NullReferenceException没有相关的消息,我会检查它是否为空,并抛出一个更好的异常消息。ConfigurationErrorException是相关的,所以我不会捕获它。

唯一的例外是如果GetConnection的“合同”并不一定在配置文件中检索连接字符串。

如果是这种情况,GetConnection应该有一个带有自定义异常的合同,说明无法检索到连接,然后您可以将ConfigurationErrorException包装在自定义异常中。

另一种解决方案是指定GetConnection不能抛出异常(但可以返回null),然后将“ExceptionHandler”添加到您的类中。


0
为什么不在添加所有防御性编程后拆分您拥有的方法呢?您有一堆独立的逻辑块,这些块需要单独的方法。为什么?因为这样您可以封装属于一起的逻辑,并且您的公共方法只需以正确的方式连接这些块。
类似这样(在 SO 编辑器中编辑,因此没有语法/编译器检查。可能无法编译;-)
private string GetConnectionString(String connectionName)
{

   //Get the connection string info from web.config
   ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];

   //documented to return null if it couldn't be found
   if (cs == null)
       throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
   return cs;
}

private DbProviderFactory GetFactory(String ProviderName)
{
   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);

   //Undefined behaviour if GetFactory couldn't find a provider.
   //Defensive test for null factory anyway
   if (factory == null)
      throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
   return factory;
}

public DbConnection GetConnection(String connectionName)
{
   //Get the connection string info from web.config
   ConnectionStringSettings cs = GetConnectionString(connectionName);

   //Get the factory for the given provider (e.g. "System.Data.SqlClient")
   DbProviderFactory factory = GetFactory(cs.ProviderName);


   //Have the factory give us the right connection object
   DbConnection conn = factory.CreateConnection();

   //Undefined behaviour if CreateConnection failed
   //Defensive test for null connection anyway
   if (conn == null)
      throw new Exception("Could not obtain connection from factory");

   //Knowing the connection string, open the connection
   conn.ConnectionString = cs.ConnectionString;
   conn.Open()

   return conn;
}

PS:这不是完全的重构,只做了前两个块。


我更喜欢自包含的逻辑。 - Ian Boyd

0
我会像你的第一次尝试一样编码。
然而,该函数的用户将使用USING块保护连接对象。
我真的不喜欢像你其他版本那样翻译异常,因为这会使查找故障原因变得非常困难(数据库宕机?没有读取配置文件的权限等)。

如果你仔细看,你会发现我实际上并没有吃掉/处理任何异常。最后,我只是防止了两种可能的空引用异常。 - Ian Boyd
好的,你是对的,我没有仔细阅读。我习惯于使用自己的连接工厂,它不会返回NULL。 - Joshua

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