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 。我可以检查是否发生了这种情况,并抛出一个异常让某人高高在上,他们给出了一个无效的连接名称:

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,但我仍然可以防御,并检查是否为空:

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 的任何提供者来决定它们抛出什么异常。抽象的开放方法文档中也没有关于如果出现错误我会发生什么的指导。如果连接出现错误,我知道我无法处理它 - 我必须让它冒泡,调用者可以向用户显示一些 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 的情况。但实际上我所做的只是将访问冲突异常(如果我尝试在空引用上调用方法)转换为InvalidArgumentException

我还发现了两种可能存在返回对象的情况;但我又一次只用一个例外换了另一个。

从积极的方面来说,它确实发现了两个问题,并解释了异常消息中发生的事情,而不是在路上发生的坏事(即责任止步于此)

但是这值得吗?这是矫枉过正吗?这种防御性编程是否出错了?

4

14 回答 14

31

手动检查配置并抛出异常并不比在缺少配置时让框架抛出异常更好。无论如何,您只是在重复发生在框架方法内部的前提条件检查,这会使您的代码变得冗长而没有任何好处。(实际上,您可能会通过将所有内容都作为 Exception 基类抛出来删除信息。框架抛出的异常通常更具体。)

编辑:这个答案似乎有点争议,所以需要详细说明:防御性编程意味着“为意外做好准备”(或“偏执”),其中一种方法是进行大量的前置条件检查。在许多情况下,这是一种很好的做法,但与所有做法一样,应权衡成本与收益。

例如,抛出“无法从工厂获得连接”异常并没有提供任何好处,因为它没有说明为什么无法获得提供者 - 如果下一行无论如何都会抛出异常提供者为空。因此,前提条件检查的成本(在开发时间和代码复杂性方面)是不合理的。

另一方面,验证连接字符串配置是否存在的检查可能是合理的,因为异常可以帮助告诉开发人员如何解决问题。无论如何,您将在下一行中获得的空异常不会告诉您丢失的连接字符串的名称,因此您的前置条件检查确实提供了一些价值。例如,如果您的代码是组件的一部分,则该值非常大,因为组件的用户可能不知道组件需要哪些配置。

防御性编程的另一种解释是,您不应该只检测错误情况,还应该尝试从可能发生的任何错误或异常中恢复。我不相信这通常是一个好主意。

基本上你应该只处理你可以做某事的异常。无论如何都无法恢复的异常应该向上传递给顶级处理程序。在 Web 应用程序中,顶级处理程序可能只显示一个通用错误页面。但是在大多数情况下,如果数据库离线或缺少某些关键配置,则无需做太多事情。

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

于 2009-06-27T17:22:24.663 回答
13

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

如果您正在编写您希望被很多其他人使用的库代码,他们不会与您讨论如何使用它,那么这并不过分。他们会感谢你的努力。

(也就是说,如果您这样做,我建议您定义比 System.Exception 更好的异常,以便那些想要捕获您的某些异常而不是其他异常的人更容易。)

但是,如果您只是自己(或您和您的伙伴)使用它,那么显然它是矫枉过正的,并且最终可能会通过降低您的代码的可读性来伤害您。

于 2009-06-27T17:23:03.277 回答
7

我希望我能让我的团队编写这样的代码。大多数人甚至没有得到防御性编程的意义。他们所做的最好的事情是将整个方法包装在 try catch 语句中,并让所有异常都由通用异常块处理!

向你致敬,伊恩。我能理解你的困境。我自己也经历过同样的事情。但是你所做的可能帮助了一些开发人员几个小时的键盘抨击。

请记住,当您使用 .net 框架 API 时,您对它有什么期望?什么看起来很自然?对您的代码执行相同的操作。

我知道这需要时间。但质量是有代价的。

PS:您真的不必处理所有错误并抛出自定义异常。请记住,您的方法只会被其他开发人员使用。他们应该能够自己找出常见的框架异常。那不值得麻烦。

于 2009-06-27T17:22:37.347 回答
6

您的“之前”示例具有清晰简洁的区别。

如果出现问题,框架最终会抛出异常。如果您对异常无能为力,您不妨让它向上传播调用堆栈。

然而,有时在框架内部深处抛出异常并不能说明实际问题是什么。如果您的问题是您没有有效的连接字符串,但框架抛出了“无效使用 null”之类的异常,那么有时最好捕获异常并使用更有意义的消息重新抛出它。

我确实检查了很多空对象,因为我需要一个实际的对象来操作,如果对象是空的,那么抛出的异常将是倾斜的,至少可以这么说。但是如果我知道会发生什么,我只会检查空对象。一些对象工厂不返回空对象;他们会抛出异常,在这些情况下检查 null 将毫无用处。

于 2009-06-27T17:33:47.317 回答
3

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

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

首先,如果应用程序配置不正确,它将无法工作。我宁愿在程序读取配置文件时知道这一点,也不愿在将来尝试创建数据库连接时知道这一点。

其次,检查配置的有效性不应该是依赖配置的对象真正关心的问题。如果您已经预先执行了这些检查,那么让自己在整个代码中到处插入检查是没有意义的。(当然也有例外 - 例如,长时间运行的应用程序,您需要能够在程序运行时更改配置并将这些更改反映在程序的行为中;在这样的程序中,您需要ConfigurationManager每当您需要设置时,请转到。)

我也不会对GetFactoryandCreateConnection调用进行空引用检查。您将如何编写测试用例来执行该代码?你不能,因为你不知道如何让这些方法返回 null - 你甚至不知道可以让这些方法返回 null。所以你已经NullReferenceException用另一个更重要的问题替换了一个问题——你的程序可以在你不理解的条件下抛出一个问题:在同样神秘的条件下,你的程序将运行你没有测试过的代码。

于 2009-06-28T19:52:50.647 回答
1

您的方法文档丢失。;-)

每个方法都有一些定义的输入和输出参数以及定义的结果行为。在您的情况下,例如:“在成功的情况下返回有效的打开连接,否则返回 null (或根据您的喜好抛出 XXXException)。牢记此行为,您现在可以决定应该如何编程。

  • 如果您的方法应该公开失败的原因和原因的详细信息,那么就像您所做的那样,检查并捕获所有内容并返回适当的信息。

  • 但是,如果您只是对打开的 DBConnection 感兴趣,或者在失败时只对null(或某些用户定义的异常)感兴趣,那么只需将所有内容包装在 try/catch 中,并在错误时返回null(或某些异常),然后返回对象。

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

于 2009-06-27T17:35:26.067 回答
1

通常,应该捕获特定于数据库的异常并将其作为更一般的东西重新抛出,例如(假设的)DataAccessFailure异常。大多数情况下,高级代码不需要知道您正在从数据库中读取数据。

快速捕获这些错误的另一个原因是它们通常在消息中包含一些数据库详细信息,例如“没有这样的表:ACCOUNTS_BLOCKED”或“用户密钥无效:234234”。如果这传播到最终用户,它在几个方面是不好的:

  1. 令人困惑。
  2. 潜在的安全漏洞。
  3. 让你的公司形象尴尬(想象一个客户用粗略的语法阅读错误信息)。
于 2009-06-27T17:42:40.910 回答
1

我的经验法则是:

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

因此, NullReferenceException 没有相关消息,我会检查它是否为 null 并抛出带有更好消息的异常。ConfigurationErrorException 是相关的,所以我没有抓住它。

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

如果是这种情况,GetConnection 应该与自定义异常签订合同,说明无法检索连接,那么您可以将 ConfigurationErrorException 包装在自定义异常中。

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

于 2009-06-28T20:35:10.717 回答
0

我会像您的第一次尝试一样对其进行编码。

但是,该函数的用户将使用 USING 块保护连接对象。

我真的不喜欢像您的其他版本那样翻译异常,因为它很难找出它崩溃的原因(数据库关闭?没有读取配置文件的权限等?)。

于 2009-06-28T20:11:27.270 回答
0

修改后的版本并没有增加太多价值,只要应用程序有一个AppDomain.UnexpectedException处理程序,它将exception.InnerException链和所有堆栈跟踪转储到某种日志文件(或者甚至更好,捕获一个小型转储),然后调用Environment.FailFast.

从这些信息中,可以相当直接地查明哪里出了问题,而无需通过额外的错误检查使方法代码复杂化。

请注意,最好处理AppDomain.UnexpectedException和调用Environment.FailFast而不是拥有顶级,try/catch (Exception x)因为使用后者,问题的原始原因可能会被进一步的异常所掩盖。

这是因为如果你捕获一个异常,任何打开的finally块都会执行,并且可能会抛出更多异常,这将隐藏原始异常(或者更糟糕的是,它们会删除文件以尝试恢复某些状态,可能是错误的文件,也许甚至重要的文件)。您永远不应该捕获表示您不知道如何处理的无效程序状态的异常 - 即使在顶级main功能try/catch块中也是如此。处理AppDomain.UnexpectedException和调用Environment.FailFast是一个不同的(更理想的)鱼壶,因为它会阻止finally块运行,如果你试图停止你的程序并记录一些有用的信息而不造成进一步的损害,你肯定不想运行你的finally积木。

于 2009-06-28T20:51:46.533 回答
0

不要忘记检查 OutOfMemoryExceptions ......你知道,它可能会发生。

于 2009-06-28T20:51:57.500 回答
0

Iain 的变化对我来说是明智的。

如果我正在使用一个系统并且使用不当,我希望获得有关滥用的最大信息。例如,如果我在调用方法之前忘记在配置中插入一些值,我想要一个 InvalidOperationException 和一条详细说明我的错误的消息,而不是 KeyNotFoundException / NullReferenceException。

这都是关于 IMO 的上下文。在我的时代,我已经看到了一些相当难以理解的异常消息,但其他时候来自框架的默认异常非常好。

一般来说,我认为最好谨慎行事,尤其是当您编写的东西被其他人大量使用或通常在调用图中更难诊断的调用图中。

我总是试图记住,作为一段代码或系统的开发人员,我比只使用它的人更能诊断故障。有时,几行检查代码 + 自定义异常消息可以累积节省数小时的调试时间(并且还使您自己的生活更轻松,因为您不会被拉到别人的机器上调试他们的问题)。

于 2009-06-28T20:52:35.423 回答
0

在我看来,您的“后”样本并不是真正的防御性。因为防御性将检查您控制下的部分,这将是参数connectionName(检查 null 或空,并抛出 ArgumentNullException)。

于 2009-06-28T20:52:52.867 回答
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:这不是一个完整的重构,只做了前两个块。

于 2009-07-02T12:17:24.187 回答