Where to use Try/Catch block?

Adam

Hi

First of all please forgive me, I seem to be having a bit of a mental block! I've been trying to improve my exception handling and error logging.

I have a function ChangeName(). Within that function I call a ValidateUser() function. And within that function I call the GetAdmin() function.

I have omitted unimportant code, so:

public void ChangeName(...)
{
        if(ValidateUser())
                //Do stuff
        else
                //log error
}

public bool ValidateUser(...)
{
        if(IsAuthenticated && GetUser(...))
                return true;
        else return false;
}

public bool GetUser(...)
{
        //Access DB and if found
                return true;
        else
                return false;
}

My question is, where should I have the try/catch blocks?
Do I just have it in ChangeName() so that all errors propogate back to this function, or do I have one in each function? I'm trying to get my head around which is best practice. I also realise that I shouldn't just catch Exception, but catch specific types first.

Thanks

5 Replies

Know the answer? Post it — somebody with the same question will find it here.

Vulpes15y ago
It's generally best IMO to do simple validation of the arguments first and, if they're invalid, throw an exception there and then or use some other approach (such as silently converting them into safe 'default' values).

A particular chore in .NET is the constant need to check that object references are not null before you call some method or property on them and, so if you can check for that first, it tends to reduce later errors.

I'd therefore revisit your methods in the light of that advice. It may cut down the need for some of your catch clauses.
0
Vilas Gite15y ago
Hi..

Really valuable explanation by Vulpes.

Refer this link also, may be helpful for you…

http://msdn.microsoft.com/en-us/library/0yd65esw(v=vs.80).aspx
http://www.csharp-station.com/Tutorials/lesson15.aspx

Thanks!

----------------------------------------------------------------------
If this reply helps your post…then check "This is correct answer"
0
Adam15y ago
Thanks Vulpes, I appreciate your advice. Taking what you've said into account, arguments are being passed from the calling methods to the called methods, so would the following be perfectly acceptable as an example?:

Class 1:
public void ProcessPerson(Request arg)
{
        try
        {
                if(Class2.ValidateUser(arg.Username, arg.Role)
                {
                         //Do something
                }
                else
                {
                        //log error
                }
        }
        catch(ArgumentNullException ex)
        {
                //do something
        }
        catch(Exception ex)
        {
                //do something
        }
}

Class 2:
public static bool ValidateUser(string Username, string Role)
{
        try
        {
                using(MyClass myClass = new MyClass())
                {
                        if(myClass.GetAdmin(Username, Role)
                                return true;
                        else
                                return false;
                }
        }
        catch(ArgumentNullException ex)
        {
                throw;
        }
        catch(Exception ex)
        {
                throw;
        }
}

MyClass (in this class I don't know whether to just let the error happen when the adapter method is called or to test the values beforehand. I think I should probably be testing the values first?:
public bool GetAdmin(string Username, string Role)
{
         // should I do this?
         if(String.IsNullOrWhitespace(Username) || String.IsNullOrWhitespace(Role))
         {
                 throw new ArgumentNullException("e.g. Username or Role cannot be null");
         }

         try
         {
                 using(MyClassAdapter myClassAdapter = new MyClassAdapter ())
                 {
                         myClassAdapter.GetAdmin(Username, Role);
                 }
         }
         catch(ArgumentNullException ex)
         {
                 throw;
         }
         catch(Exception ex)
         {
                 throw;
         }
}

Appreciate any advice or if anyone else wants to comment?

Thanks.
0
Vulpes15y ago
I'd just like to expand a little on my earlier post.

Sometimes, even though it's caught an exception, the method in which it's thrown is not best placed to take remedial action - perhaps because the exception is a consequence of arguments that were passed to it by the calling method - and in this case it should be rethrown so that an exception handler earlier in the chain can hopefully catch and do something about it.

This is particularly relevant where you're writing class libraries for others and therefore have little control over the arguments that may be passed to a particular method.

Circumstances can also occur where there are so many exceptions that may occur that it's just not practicable to catch them all in one place. It may therefore be best to leave some of these exceptions to be handled by a generic exception handler for the thread as a whole.

Finally, there are some exceptions which are fatal (hopefully, not too plentiful in production applications!) and these are best left to be caught and logged by 'last resort' handlers before the application is allowed to terminate in a graceful fashion.
0
Vulpes15y ago
As a general rule, you should try and catch exceptions in the method where they're thrown because:

1. That's usually the best place to be able to do something about them.

2. It saves methods earlier in the call stack from having to place try/catch around each method call they make.

3. In some cases, the calling method may not even know what exceptions the called method may throw because, unlike Java, these don't have to be declared in C#.
0