Why do try blocks need a catch
Asked Answered
C

10

6

I have this code :

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.

I know the importance of exception handling, but here is simply nothing exceptional about the exception and nothing I can do (nor need to do) to fix the behavior.

Cass answered 15/3, 2012 at 2:18 Comment(2)
Read this [post][1] [1]: #1573630Mammalian
@Mammalian - Not sure how that's relevant to my question?Cass
N
7

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.

Nephritic answered 15/3, 2012 at 2:25 Comment(1)
Now this is the first reasonable answer I have seen. I didn't consider the case of what happens if its another type of exception to the one I'm swallowing (although not really relevant in my example the point is definitely taken) Thank you for your input.Cass
W
6

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).

Wellstacked answered 15/3, 2012 at 2:21 Comment(0)
W
4

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 ... */
   }
}
Waldron answered 15/3, 2012 at 2:22 Comment(4)
Because sharepoint XML webservices have the wonderful behaviour of simply omitting the attributes that are null rather then providing an empty value.Cass
Unfortunately not, the attribute collection will typically contain at least one value because you've got to remember that typically you would be returning at least a few columns. This probably would work if you were just returning one column. Hopefully that makes sense.Cass
This will work, as you can always check for multiple columns ... I don't particularly understand where your confusion is ...Waldron
My bad after going back and checking you're infact correct. Your solution does work. I do apologize.Cass
S
3

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'

Strawworm answered 15/3, 2012 at 2:24 Comment(1)
Sharepoint XML webservices simply omit the attributes that are null rather then providing an empty value. Which would mean that for every subset of data (IE: Every row) I would need to run through the data and check. Simply allowing the exception to occur (although debatable) is much more practical in my opinion.Cass
C
2

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?

Clang answered 15/3, 2012 at 2:22 Comment(1)
Please see my response to Simon Wang and Xander re my specific codeCass
R
1

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.

Rissole answered 15/3, 2012 at 2:21 Comment(0)
T
1

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).

Tourist answered 15/3, 2012 at 2:22 Comment(1)
Presumably he's asking why this is the syntax.Wellstacked
T
1

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?

Teethe answered 15/3, 2012 at 2:23 Comment(0)
P
1

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.

Proposition answered 15/3, 2012 at 2:25 Comment(1)
I don't believe there is a way to check with the XmlAttributeCollection (I have looked but could be wrong) but even if there was I would still argue that there are scenarios where this syntax would be appropriate.Cass
R
0

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

Reminds me of Classic ASP On Error Resume Next

Reid answered 15/3, 2012 at 2:24 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.