CODEWITHSUNDEEP
c#.netexceptionif-statement
Peter
Peter

Reputation: 207

Exception or If/Else statement

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

Answers (5)

SWeko
SWeko

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

mmix
mmix

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

Jaapjan
Jaapjan

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

Russell Troywest
Russell Troywest

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

Jon Skeet
Jon Skeet

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

Related Questions