Question

I have a bit of code as such

try
{
    result.FirstName = nodes[myIdx].Attributes["ows_FirstName"].Value;
} catch { }

Now I don't know prior to invoking this call if the attribute I am looking for exists (Good ol sharepoint).

As a result the only linear way I can write the code I am looking to create is as such.

try
{
    result.FirstName = nodes[myIdx].Attributes["ows_FirstName"].Value;
} catch { }
try
{
    result.LastName = nodes[myIdx].Attributes["ows_LastName"].Value;
} catch { }

....

Now I have no use for the catch section of this code and end up with a huge amount of lines that are completely redundant.

Why couldn't I just do

try { result.FirstName = nodes[myIdx].Attributes["ows_FirstName"].Value; }

So why are we explicitly forced to declare a catch block even when it isn't handled? I'm sure there is a good reason but can't work it out.

EDIT: Before everyone starts going off at me that swallowing an exception is bad, blah blah blah. We (and I) all know those arguments but in this (and many) real world scenarios there is simply nothing exceptional about the exception and nothing I can do (or need to do) to fix the behaviour.

Was it helpful?

Solution

They are not redundant - they have a specific purpose. By default the lack of a catch block will re-throw the exception to the calling method. An empty catch block essentially "swallows" the exception and lets the program continue, oblivious to whether or not an exception was thrown; generally a bad practice.

there is simply nothing exceptional about the exception

it may be true that one type of exception may not be "exceptional" in this case, but that's not the only exception that can occur. You should handle that exception and deal with any others appropriately.

For example -

What if nodes is null? What if myIdx is outside the bounds of the nodes array? Either of these conditions would be exceptional and you should either handle them specifically or let the calling program handle them and act appropriately.

[there's] nothing I can do (or need to do) to fix the behaviour

You may not be able to fix it, but you may need to be aware of it. How does the program behave differently in that case? Log a message? Raise a warning? Set a default value? Do nothing may be an appropriate answer, but very likely not an appropriate response to any possible exception.

OTHER TIPS

If you want to swallow an exception (catch it but do nothing), you have to explicitly do so.

This is usually poor practice, so there is no reason to provide a syntactic shortcut. You should generally either:

  1. Handle the exception in some way. This might mean:
a. Retry
b. Rethrow it (preserving the inner exception) with a more meaningful message.
c. Do it another way.
d. Log it (though logging and rethrowing might be better).
e. Other

2. Just let it bubble up (no try, or just try/finally).

Why not just check if the item isn't null?

if(nodes[myIdx].Attributes != null &&
   nodes[myIdx].Attributes["ows_FirstName"] != null) {
    /* ... your code ... */
}

Or:

if(nodes[myIdx].Attributes != null) {
   if(nodes[myIdx].Attributes["ows_FirstName"] != null) {
       /* ... your code ... */
   }
   if(nodes[myIdx].Attributes["ows_LastName"] != null) {
       /* ... your code ... */
   }
}

There must be another way to check if that Attributes exist or not, you should not using exception handling for some functional purpose, you code like this:

try
{
    newstring = oldString.ToString();
}
catch{}

what you should do is:

if(oldString != null)
{
    newstring = oldString;
}

Remember that try catch is for handling things named as 'exception'

The reasoning is probably because you shouldn't be catching an exception if you can't handle it. Allowing you to try without a corresponding catch would do nothing but enable a worst practice.

As for the specific code you're referencing, couldn't you just do a null check prior to trying to access the indexer?

The try/catch block was designed Explicitly for catching and handling Exceptions that are thrown in your application.

Simply swallowing an Exception is typically a bad idea (even if you're just logging the Exception) and must be done explicitly.

This has pretty much been answered in this question: C#: should all exceptions be caught

Basically, if you are catching it, then you should do something with it, otherwise dont catch it. In your case, why can't you check if the attribute value exists first?

It looks like you are using one of the SharePoint Web Services, so the return type is some sort of XmlElement? I'm pretty sure there is a way to check if an attribute exists, which is cheaper than throwing an exception.

Also, you way want a helper method which encapsulates the check and data retrieval.

It's part of the language syntax. You cannot try without at least one catch, or a finally. There is no standalone try, only try-catch, try-finally, and try-catch-finally.

It simply does not make sense to try some code if you're not going to bother handling an exception (catch), or at least making sure some subsequent code always runs regardless of what happened (that's finally).

You should not have empty catch blocks. That is poor programming practice.

Reminds me of Classic ASP On Error Resume Next

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top