Reputation: 207
I have a question about when to use an Exception or If/Else statement. In my situation i want to check if a DocumentNode is a TestNode. When it is a TestNode, I want to get the node.
I wrote two possible solutions for this below. The first solution thinks it is a TestNode and else it gives a Exception. The second solution checks if it is a TestNode, then it executes approximately the same function to get the node. Can anyone tell me what is the best solution? Or is there a better solution for this? Thanks, Peter.
*Sorry for my bad English..
private DocumentNode GetTestNode(IEnumerable<DocumentNode> nodes)
{
foreach (DocumentNode node in nodes)
{
if (node.GetValueProperty().ToString() == "TestValue")
{
return node;
}
}
throw new TestNodeNotFoundException("This is not a TestNode");
}
OR:
private DocumentNode IsTestNode(IEnumerable<DocumentNode> nodes)
{
foreach (DocumentNode node in nodes)
{
if (node.GetValueProperty().ToString() == "TestValue")
{
return true;
}
}
return false;
}
private DocumentNode GetTestNode(IEnumerable<DocumentNode> nodes)
{
foreach (DocumentNode node in nodes)
{
if (node.GetValueProperty().ToString() == "TestValue")
{
return node;
}
}
}
Upvotes: 2
Views: 1167
Reputation: 30942
A smart rule of thumb is: Exceptions should be exceptional.
Throwing or not throwing an exception should not be that much of a code issue, as a logic issue. What is your context? Is it the end of the world if DocumentNode
collection does not contain a test node? Can you proceed your processing if that is the case? It the test node is mandatory, and it will always be included, except in some exceptional circumstances, than, yes, do throw an exception.
But if the test node is optional, or is often missing, in general, if the scenario where there is no test node is not that unusual, then exceptions are a overkill, since nothing exceptional happened.
Upvotes: 0
Reputation: 6308
You are in a .net land now :) Why not go the Linq way? Create an extension yielding IEnumerable<DocumentNode>
on IEnumerable<DocumentNode>
? This will later allow you to use other Linq operations on a subset of test nodes
public static IEnumerable<DocumentNode> GetTestNodes(this IEnumerable<DocumentNode> nodes)
{
return nodes.Where(x => x.GetPropertyValue().ToString() == "TestValue");
}
...
docnodes.GetTestNodes()....
Upvotes: 1
Reputation: 3385
Generally, your program should handle errors gracefully.
Throwing and Exception
should be done when the code has run into an unsurmountable problem and immediately stops, throwing the error conditions higher up the callstack in the hope someone else will be able to recover from the condition.
Upvotes: 0
Reputation: 8776
It depends. Is not having a testnode an exceptional circumstance that requires execution of that function to be halted and error handling to be done?
If so, then - yes, use an exception. If it's a fairly normal thing to happen and something you expect to occur then an exception is probably not the way to handle it.
Upvotes: 1
Reputation: 1504182
A better solution would be to use LINQ if you can. If you want an exception, you can use:
var node = nodes.Where(n => node.GetPropertyValue().ToString() == "TestValue")
.First();
(You can also use Single
if there should be exactly one such node.)
If you don't want an exception, use FirstOrDefault
instead:
var node = nodes.Where(n => node.GetPropertyValue().ToString() == "TestValue")
.FirstOrDefault();
Then node
will be null if there are no TestValue nodes.
To test for the presence of a test node, use Any
var isTest = nodes.Where(n => node.GetPropertyValue().ToString() == "TestValue")
.Any();
(Your final method won't currently compile, as it can reach the end without returning.)
Which is appropriate really depends on what you're trying to do. If the lack of a test node indicates a bug, then throwing an exception is appropriate. If not, using null
to signal that is pretty reasonable, or you could use the TryXXX
pattern to explicitly return a bool
value, while saving the found node (if any) in an out parameter.
Upvotes: 7