我应该多余地测试参数(例如集合为空)吗?
这里冗余集合检查有问题吗:
SomeMethod()
{
shapes = GetShapes();
//maybe Assert(shapes.Any())?
if(shapes.Any())
{
ToggleVisibility(shapes);
}
}
ToggleVisibility(IEnumerable<Shape> shapes)
{
//maybe Assert(shapes.Any())?
if(shapes.Any())
{
//do stuff
}
}
Is there a problem with the redundant collection checking here?:
SomeMethod()
{
shapes = GetShapes();
//maybe Assert(shapes.Any())?
if(shapes.Any())
{
ToggleVisibility(shapes);
}
}
ToggleVisibility(IEnumerable<Shape> shapes)
{
//maybe Assert(shapes.Any())?
if(shapes.Any())
{
//do stuff
}
}
如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。
绑定邮箱获取回复消息
由于您还没有绑定你的真实邮箱,如果其他用户或者作者回复了您的评论,将不能在第一时间通知您!
发布评论
评论(6)
我认为这里没有什么大问题,因为调用 Any() 并不是一个昂贵的操作。
有一个小问题是 ToggleVisibility 的责任和行为没有声明。 ToggleVisibility 应该让调用者知道形状为空或 null 时它将如何表现。做到这一点的最佳方法是通过 XML 注释,以便它显示在 Intellisense 中。这将使 ToggleVisibility 调用者决定是否需要检查集合是否为空或 null。
I don't think there's a big problem here because calling Any() is not an expensive operation.
There is a minor problem in that the responsibility and behavior of ToggleVisibility is not declared. ToggleVisibility should let callers know how it will behave if shapes is empty or null. The best way to do this is through XML comments so that it shows up in Intellisense. This will let ToggleVisibility callers decide if they need to check if the collection is empty or null.
如果您添加这些断言用于测试和调试,那么当然这是有意义的。
在这些情况下,当事情没有按照您期望的方式发展时,您希望得到通知。
然而,在生产中,您可能不希望通过调用形状集合中不存在的成员来破坏整个应用程序。
If you're adding those assertions for testing and debugging, sure, that makes sense.
In those situations, you want to be told when things don't go the way you expect them always to go.
In production, however, you probably don't want to tank the whole application by making calls on non-existent members of the shapes collection.
您可以使用 代码合同 图书馆。在这种情况下,您可以在代码中动态配置前置条件(验证传入值)、后置条件(验证结果)和不变量(对于特定类必须始终为真的条件)。
You can use Code Contract Library. In this case you can dynamically configure preconditions (validating incoming values), postconditions (validating results) and invariants (conditions that must be always true for a particular class) in your code.
我认为这里的关键是认识责任。如果您知道每个会调用 ToggleVisibility 的地方并且打算始终事先进行检查,那么最好不要检查 ToggleVisibility 方法。
就我而言,我会在 ToggleVisibility 中检查它,因为它使调用者代码更清晰,如果您从 50 个不同的位置调用 ToggleVisibility 函数,那么您的代码就会少得多。
I think the key here is knowing the responsibility. If you know every single place that will ever call ToggleVisibility and intend to always check before hand then it is fine to not check in the ToggleVisibility method.
For my part I would check it inside ToggleVisibility because it makes the caller code cleaner and if you call the ToggleVisibility function from 50 different places then you have considerably less code.
我建议答案是......像往常一样......“这取决于”。虽然在 IEnumerable 上调用 Any 并不昂贵,但真的有必要吗?这取决于您计划在该方法中处理您的集合。
您的方法是否会因为集合为空而引发异常或其他不需要的内容?您是否使用 foreach 迭代您的集合?如果是这样,那么拥有一个空集合不一定会造成任何损害,尽管它可能违反您的业务规则。尝试迭代 null 集合显然是不同的。
您使用
GetShapes()
作为答案的示例框架。为了扩展我的想法,在空集合上使用ToggleVisibility()
真的违法吗?它显然不会做太多事情,但如果用户突出显示一组空的形状,然后单击切换可见性功能,它会做任何坏事吗?I would suggest that the answer is... as usual... "it depends". While calling Any on an IEnumerable is not expensive, is it really necessary? That depends on what you are planning on doing with your collection in the method.
Will your method throw an exception, or something else undesirable, because of an empty collection? Are you iterating over your collection with a foreach? If so, then having an empty collection wouldn't necessarily do any harm, though it may be against your business rules. Trying to iterate over a null collection is obviously different.
You use
GetShapes()
as an example framework for an answer. To expand on my idea, is it really illegal toToggleVisibility()
on an empty collection? It obviously won't do much, but if the user highlighted an empty set of shapes, and then clicked on the toggle visibility function, would it do anything bad?如果),并且其中任何一个地方都可能有一个空集合,那么我肯定会减轻调用者每次进行检查的负担,并将其粘贴到方法本身中。
ToggleVisibility(IEnumerable)
是一种私有方法(因此SomeMethod()
必须位于同一个库中),那么我肯定只会在其中包含一次检查发布版本。检查是否采用一种方法或另一种方法取决于对于正在发生的事情来说什么是有意义的。如果在正确的执行中预计集合永远不会为空,那么也许不需要检查。如果从十个不同的地方调用 ToggleVisibility(IEnumerable如果
ToggleVisibility(IEnumerable)
是公共 API 的一部分,那么它绝对应该执行任何必要的参数验证,因为 API 的用户可能会执行任何操作,并且必须在以下位置检查所有参数所有的时间。如果该方法的文档声明将忽略空集合,那么显然不需要担心SomeMethod()
。否则,SomeMethod()
需要尽一切努力来验证它传递的集合是否有效,即使这意味着要进行冗余检查。If
ToggleVisibility(IEnumerable<Shape>)
is a private method (thusSomeMethod()
must be in the same library), then I would definitely include the check only one time in the Release build. Whether the check is in one method or the other depends on what makes sense for what is happening. If the collection is expected to never be empty in a correct execution, then perhaps no check is needed. IfToggleVisibility(IEnumerable<Shape>)
is being called from ten different places, and any of them may have an empty collection, then I would definitely relieve the caller of the burden of doing the check every time, and just stick it inside the method itself.If
ToggleVisibility(IEnumerable<Shape>)
is part of a public API, then it should definitely do whatever parameter validation is necessary, since users of APIs are likely to do anything, and all parameters must be checked at all times. If the documentation for the method states that empty collections will be ignored, thenSomeMethod()
does not need to worry about it, obviously. Otherwise,SomeMethod()
needs to do whatever it takes to verify that the collection that it is passing is valid, even if that means that redundant checks are made.