logo标签列表

重新抛出包装对象的异常

c#.netexceptiontry-catch
3

我使用 ConcurrentDictionary<TKey, TValue> 来实现 ConcurrentSet<T>。

public class ConcurrentSet<T> : ISet<T>
{
    private readonly ConcurrentDictionary<T, byte> collection;
}

ConcurrentDictionary<TKey, TValue> 无法包含具有空键的键值对。

// summary, param, returns...
/// <exception cref="ArgumentNullException">
///     <paramref name="item" /> is null.
/// </exception>
public bool Add(T item)
{
    // This throws an argument null exception if item is null.
    return this.collection.TryAdd(item, 0);
}

所以，我应该...
if (item == null)
    throw new ArgumentNullException("item", "Item must not be null.");

return this.collection.TryAdd(item, 0);

或者，我应该：

try
{
    return this.collection.TryAdd(item, 0);
}
catch (ArgumentNullException)
{
    throw new ArgumentNullException("item", "Item must not be null.");
}

或者，我应该，
try
{
    return this.collection.TryAdd(item, 0);
}
catch (ArgumentNullException x)
{
    // I know, but I don't want to preserve the stack trace
    // back to the underlying dictionary, anyway.
    throw x;
}

或者，我应该，
try
{
    return this.collection.TryAdd(item, 0);
}
catch (ArgumentNullException)
{
    // The thrown exception will have "key", instead of
    // "item" as the parameter's name, in this instance.
    throw;
}

这个怎么正确的做呢？
看看这个关于重新抛出异常的SO答案：https://dev59.com/KXNA5IYBdhLWcg3wpfmu - George Johnston
1
请问能否添加关于使用ArgumentNullException包装ArgumentNullException的意义的信息？ - Ilya Ivanov
1
@IlyaIvanov：我想这个想法是因为根本问题是使用空值调用了 ConcurrentSet.Add，所以堆栈跟踪应该显示这一点。此外，抛出的异常应该表明空参数的名称是 "item" 而不是 "key"。 - supercat
@IlyaIvanov：就像supercat所说的那样，由ConcurrentDictionary<TKey, TValue>引发的ArgumentNullException将其ParamName属性设置为“key”，而不是“item”。这不是此类的预期行为，它可能会使ConcurrentSet<T>的使用者感到困惑，因为它通过异常的ParamName属性和源自ConcurrentDictionary<TKey, TValue>的堆栈跟踪公开了实现细节。 - Şafak Gür
@wonko79：这不是你提到的问题的重复，因为这更多关于API设计而不是错误处理。ConcurrentSet<T>的使用者应该在将null值传递给Add(T)方法时处理ArgumentNullException，但他们是否应该知道它是由并发字典引发的，还是堆栈跟踪应该指向Add(T)而不是TryAdd(TKey, TValue)？哪一个更糟糕，验证一个将被包装类验证的参数，还是让第二个异常初始化？ - Şafak Gür
4个回答
7
我建议选择这个。
public bool Add(T item)
{
    // This throws an argument null exception if item is null.
    return this.collection.TryAdd(item, 0);
}

或者这个

if (item == null)
    throw new ArgumentNullException("item", "Item must not be null.");

return this.collection.TryAdd(item, 0);

取决于您的班级是否在意空值。如果您执行空值检查的唯一原因是为了避免将空值传递给“TryAdd”，那么不要费心进行检查。“TryAdd”会自行检查并抛出异常。
如果您认为将来可能会使用允许空值的不同集合，但仍希望您的集合不包含空值，则应进行检查。这将保护您，以防未来发生变化。
参数验证应始终是方法执行的第一件事情。如果参数无效，则做任何其他事情都没有意义。
只有在需要处理异常时才应捕获异常。如果您只是重新抛出或创建一个等效的新表达式，则不必捕获它。
我会选择这两个中的第二个 - 因为堆栈跟踪将从错误点附近开始（即将null传递给外部类的Add()而不是内部集合的Add()，后者是一种实现细节）。 另外，我会使用`Contract.Requires（item！= null）'，但那是一个完全不同的问题。 - Matthew Watson
如果你执行 null 检查的唯一原因是为了避免将 null 传递给 TryAdd，那么不必检查。TryAdd 将执行自己的检查并抛出异常。确切地说，它抛出的 ArgumentNullException 异常将其 ParamName 属性设置为 "key"，因为它是由 ConcurrentDictionary<TKey, TValue> 抛出的，这是我疑惑的主要原因。是的，我包装了 ConcurrentDictionary<TKey, TValue>，但这是实现细节，我更喜欢不暴露它。对于消费者来说，它应该只是一个线程安全的哈希集合，不允许空值。 - Şafak Gür
1
我会在这里使用第二个选项，以确保ParamName属性是正确的。我还会使用 throw new ArgumentNullException("item"); ，因为额外的消息是一个不必要的明显信息重复（这个ArgumentNullException构造函数的单个字符串参数是参数名称）。 - Sam Harwell
1
你需要做的取决于你想记录你的类在做什么。如果你希望记录尝试添加空项可能以未指定的方式失败，那么只需直接调用并让任何异常冒泡即可。如果你希望记录将返回一个ArgumentNullException，其中ParamName等于item，并且不希望依赖于当ConcurrentDictionary接收到空键时的行为，则应在传递给ConcurrentDictionary之前检查该参数。如果你希望记录你的代码将抛出一个ArgumentNullException，其中ParamName等于item，但愿意依赖于ConcurrentDictionary来验证其参数并抛出一个ArgumentException，并且如果性能至关重要，另一个可能性是：
try
{
    return this.collection.TryAdd(item, 0);
}
catch (ArgumentNullException ex)
{
    if (ex.ParamName == "key" && item == null)
        throw new ArgumentNullException("item", "Item must not be null.");
    else
        throw;
}

这段代码避免了在参数不为空的情况下（这应该99.9999%的情况下都是成立的）对参数进行额外的验证所产生的任何额外费用，但是仍然会确保只有在发生预期原因导致的ArgumentNullException异常时才会声称是其源头；如果ConcurrentDictionary中的一个bug导致它即使在给定非空项以添加时也会无意中将null参数传递给内部调用的方法，则上述代码将确保不会丢失原始异常堆栈跟踪。请注意，另一种可能性可能是：

    if (ex.ParamName == "key" && item == null)
        throw new ArgumentNullException("item", "Item must not be null.");
    else
        throw new UnexpectedException(ex); // Probably a custom type

基本思想是，如果一个 ArgumentNullException 从 ConcurrentDictionary.Add 中逃脱出来，除了 item 为空之外的某些其他原因，这样的异常不应该被可能期望你抛出 ArgumentNullException 的代码捕获。
+1，很棒你也解决了多重验证问题。谢谢。 - Şafak Gür
@ŞafakGür：值得一提的是，参数验证的执行时间影响可能非常小，因为引用在使用之前很可能必须加载到寄存器中，但是.NET的代码生成工具不是生成在使用引用之前测试空值的代码，而是设置运行时环境，以便空引用将触发硬件陷阱，然后转换为“NullReferenceException”。框架这样做的事实表明，避免空测试有一定的性能优势。 - supercat
哇，感谢您提供的详细信息。说实话，在您提到它对性能的影响之前，我从未考虑过这个问题。我的动机相当原始，只是觉得验证某些东西并调用一个将执行相同验证并抛出几乎相同异常的方法是错误的，但是了解具体情况还是很好的。 - Şafak Gür
@ŞafakGür：我的个人偏好是指定例程应该在提供无效数据时执行必要的操作以维护不变量，但它们可以随意抛出任何异常，除了那些被定义为表示其他含义的异常。Java的已检查异常几乎走在正确的轨道上，只是指定未预期的已检查异常应该升级为未检查异常应该不比指定它们应该向调用者冒泡更难（前者行为更常见）。 - supercat
1
我认为，你应该做什么取决于你想要的效果。如果你想吞掉错误并不向用户显示它，那么在catch块中不要重新抛出错误，但要包含try-catch。如果你想要自定义错误消息，则使用"Item == null"检查。生成新的异常实例没有太大用处，所以也不需要这样做。
至于其余内容...如果你没有记录错误或在更上游特别处理它，那么在捕获后重新抛出错误是没有必要的。否则，这取决于个人风格和是否需要自定义错误消息。
我的最爱可能是带有自定义错误消息的"Item == null"检查，但这更多是因为我喜欢自定义错误消息。我发现它们对我更有用，但确保调用此方法的内容周围有错误处理，以免错误导致更上游的未处理异常。
+1 for insight。因此，为了使用自定义错误消息并隐藏包装的ConcurrentDictionary<TKey, TValue>（这只是一个实现细节），您建议在调用字典方法之前使用自己的错误处理，并确保它不会引发异常。然后，在try-catch中调用它以防万一，然后再从catch块中抛出自定义异常，对吗？我困惑的原因是字典已经执行了此验证。所以我想也许我应该让它自己完成，如果它引发异常，则抛出自定义异常而不是暴露那个异常。 - Şafak Gür
实际上，在try catch中调用字典的add方法并不是必须的，除非你真的想要这样做，但这并不是一个坏主意，因为毕竟它是一个字典... 但我所指的是在方法之外的部分，当你调用那个方法时，确保你在某个级别上处于try catch块中，否则你刚才抛出的自定义异常将导致上游未处理的异常错误。 - Nevyn
0
在你的例子中，它们都太相似了，但我会选择第一个选项：
if (item == null)
    throw new ArgumentNullException("item", "Item must not be null.");

因为它不需要 catch，看起来更紧凑。如果需求发生变化，它还可以让您扩展条件，而不会增加更多的代码行，例如

if (item==null || item.Name == null)
    throw...
@ŞafakGür 不好意思，我之前举的例子有误。通常情况下，TryXYZ不会对异常进行处理，但这次不同。 - Sten Petrov
网页内容由stack overflow 提供, 点击上面的
可以查看英文原文，
原文链接