Is this the case for empty catch catch blocks?

Suppose you are reading in a large XML file and about 25% of the nodes are optional, so you really don't care if there are any, however, if provided to you, you would still read them in and do something with them ( e.g. store them in db).

Since they are optional, in this case, wrap them in empty blocks try . . . catch

, so in case they are not there, the program just continues execution? You don't care about throwing an error or anything like that.

Keep in mind that just because they are optional doesn't mean you don't want to test them. It simply means that the person providing the XML to you either doesn't want you to know certain information, or they would like you to know and it's up to you how to do it.

Finally, if it were just a few nodes, it wouldn't be a big problem, but if you have 100 nodes that are optional, for example, it can be painful to check if each is the null

first one, or to terminate execution if found null

so the reason I asked if this is a valid reason for empty catch catch statements.

+3


source to share


3 answers


If node X processing is optional, then your code should look something like this:

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 tell if node X exists other than trying to use it, or if possible, to remove node X after checking to see if it is there, is forced to use the try / catch model. There is no situation here. (In contrast, by checking if a file exists before reading it, someone can delete it after checking to see if it is there.)

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

Edit:



Since it seems like your problem is accessing the "grandchild" node only in the following XML, where the "parent" may not exist. (Please excuse my disadvantageous ability to display this XML in SO; knowledgeable readers feel free to edit in the appropriate format.)

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

      

For this I would 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
}

      

+5


source


Overall, the scenario sounds like a borderline acceptable use case for an empty catch block (assuming, of course, that catch is bound to the type of exception that was thrown when there was no node). Since you have no work to do, if there is no node, then the code execution will continue as planned.

I doubt if the pain of checking is null

too much for too much. The amount of code / pain to check for zero is 2 lines

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

      

Overhead try / catch

, albeit just as detailed



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

      

Given this choice, I would take the approach if

. It is equally declarative, generally speaking a faster and more general style. Exceptions should only be used for exceptional situations. Material that cannot be prevented in front of hand. This is a very preventable situation and could potentially be even more acceptable with an extension method to support the pattern

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

      

+2


source


Since they are optional, in this case, wrap them with empty try-capture blocks so that even if there are none, the program just continues.

No - instead, you should check if a particular node exists before doing anything else. Since you expect this to be the case sometimes, your programming logic should cover this, it is not used to handle exceptions.

+1


source







All Articles