CODEWITHSUNDEEP
c#exception
Xaisoft
Xaisoft

Reputation: 46591

Is this a case for empty try catch blocks?

Let's assume that you read in a large XML file and about 25% of the nodes are optional, so you really don't care if they are there, however, if they are provided to you, you would still read them in and do something with them (store them in a db for example).

Since they are optional, isn't it OK in this case to wrap them in empty try . . . catch blocks, so in the event that they are not there, the program just continues execution? You don't care about throwing an error or anything similar.

Keep in mind that just because they are optional does not mean you don't want to check for them. It just means that the person providing you with the XML either doesn't want you to know certain information or they do want you to know and it is up to you to handle it.

Finally, if this was just a few nodes, it wouldn't be a big deal, but if you have 100 nodes that are optional for example, it can be a pain to check if each one is null first or halting execution if a null was found, hence the reason why I asked if this is valid reason for empty try catch statements.

Upvotes: 3

Views: 507

Answers (3)

Servy
Servy

Reputation: 203814

If processing node X is optional then your code should looks something like:

if(node X exists in file)
{
  do work with X
}

and not:

try
{
  do work with X
}
catch{}

Now if there is no way to determine if node X exists other than to try to use it, or if it's possible for node X to be removed after you check to see if it's there, then you're forced to use the try/catch model. That isn't the situation here. (As opposed to say, checking if a file exists before reading it; someone can delete it after you check to see if it's there.)

------------------------------------------------------------

Edit:

Since it seems your issue is to access node "grandchild" alone in the following XML in which 'Parent' may not exist. (Please excuse my poor ability to render this XML in SO; knowledgeable readers feel free to edit in the appropriate formatting.)

<root>
  <Parent>
    <Child>
      <GrandChild>
        The data I really want.
      </GrandChild>
    </Child>
  </Parent>
</root>

For that I'd do something like this:

public static class XMLHelpers
{
public static Node GetChild(this Node parent, string tagName)
{
  if(parent == null) return null;
  return parent.GetNodeByTagName(tagName);
}
}

Then you can do:

var grandbaby = rootNode.GetChild("Parent").GetChild("Child").GetChild("GrandChild");
if(grandbaby != null)
{
  //use grandbaby
}

Upvotes: 5

JaredPar
JaredPar

Reputation: 754565

In general the scenario sounds like a borderline acceptable case for using an empty catch block (assuming of course the catch is scoped to the type of the exception thrown when there is no node). Since you don't have any work to do if the node isn't present then the code execution will continue as planned.

I do question whether or not the pain of checking for null is too great here. The amount of code / pain to check for null is 2 lines

if (parent.IsPresente("child")) {
  var child = parent.GetNode("child");
}

The overhead of a try / catch though is just as verbose 

try { 
  DoSomething(parent.GetNode("child"));
} catch (TheExceptionType) { }

Given this choice I would choose the if approach. It's equally declarative, generally speaking faster and overall a better style. Exceptions should really only be used for exceptional situations. Stuff which can't be prevented before hand. This is a very preventable situation and could potentially even be made more palatable with an extension method to support the pattern

XmlNode child;
if (parent.TryGetNode("child", out child)) {
  ..
}

Upvotes: 2

BrokenGlass
BrokenGlass

Reputation: 160852

Since they are optional, isn't it OK in this case to wrap them in empty try catch blocks so in the even that they are not there, the program just continues execution.

No - instead you should check whether the particular node exists before doing anything else. Since you expect this to be the case sometimes, your program logic should cover this, this is not a use case for exception handling.

Upvotes: 1

Related Questions