Where to use Try/Catch block?

Question

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

Vulpes · Answer

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.

Vilas Gite · Answer

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

Adam · Answer

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.

Vulpes · Answer

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.

Vulpes · Answer

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