Where to implement try catch blocks?

I am always struggling where to place a try catch block. For example, I have a database class with a method that accepts two parameters. FindObject(string where, string order). This method executes a sql query with the specified where and order strings.

In a class I have a property called IsUsed, this property looks like this:

public bool IsUsed 
{  
 get
 {   
  ClassA a = new ClassA();   
  Collection<ClassA> myCollection = a.FindObject("Id = 1","");

  if(..) // etc  
 } 
}

It doesn't matter if this approach is clever or not, I only want to know whe开发者_如何学Cre to place the try catch if the execution of the sql query goes wrong.

Where should I place the try catch so I can notify the user something went wrong?

1. In the FindObject method?
2. In the IsUsed property?
3. Where I call the IsUsed property?
4. Somewhere else? But where

I try to follow a fairly simple rule: I set up a try..catch block if I can handle the exception in a sensible way. If there is nothing meaningful that I can do about the exception, I let it bubble to the calling code.

As a side note I would avoid executing (potentially lengthy) database calls in a property getter. I typically try to have the properties just set or get the value from a backing field, letting other methods perform database lookups and such. That will make the code somewhat more predictable for the person writing the calling code (it's common for property access to be a cheap action, while method calls can often be more expensive).

Only catch the exception if you can do something about it. Otherwise catch exceptions at the "highest" level in your application and do whatever is required to handle it including terminating your application.

In applications having a UI the "highest" level is often event handlers like the click handler for a search button. For background services the "highest" level is often thread procs or timer callbacks.

  UI application                          Background service

| System Code    |                      | System Code    |
+----------------+                      +----------------+
| Event Handler  | <- try/catch here -> | Thread proc    |
|                |                      |                |
| ...            |                      | ...            |
|                |                      |                |
| Method         | <- throw here     -> | Method         |

If you let the exception propagate back into system code you will have an unhandled exception and you application will crash.

Handling an exception in a UI application often involves showing a message box. Some exceptions are not fatal and the operation may be retried (say if a file is missing or a database query failed) but other exceptions are fatal and the only option is to terminate the application.

A background service will log the exception and perhaps retry the operation. If several retries fail the logging level may increase to get the operators attention.

Good practice when it comes to exception handling:

• If you catch an exception and rethrow you own exception wrap the original exception as the InnerException of the new exception.
• If you catch an exception perhaps to do some cleanup but rethrow it because you want it to bubble up then always rethrow it using throw without specifying any exception. This will ensure that the original stack trace isn't destroyed.
• In most cases you should only ever catch the base class Exception in your top level handlers.
• Use finally blocks or even better the IDisposable pattern to perform proper cleanup in your code.
• Think of exceptions as the "user interface" to the developer and format the exception messages accordingly. If you need a more polished user interface for the less technical user you should probably hide the more technical stuff.
• Try to only use exceptions for exceptional situations like unexpected errors. Don't throw exceptions for common error cases. Checking the existence of a key in a dictionary should not throw an exception but instead return true/false value.

To answer your specific question I don't think you should catch any exceptions in your property.

You should place try-cacth block in the place where you can do something meaningful with caught exception. Like you log it or show it to the user. Do not catch exceptions just for the sake of it.

Well it should be where the try..catch block is needed, and where it can be handled. I'm assuming this would be in the FindObject method here.

So, within the FindObject method, catch and handle the SqlException, for instance. If it needs to be moved to a higher level for better handling, then throw the exception, or let it simply bubble up.

It's easily-answerable: Where you can deal with it appropriately.

I.e, at the given place, can you make a useful decision on the basis of the catch? Can you return something else? Can you tell someone something? Can you translate the error to a more useful error to some other layer of your application?

If the answer is yes, then you catch it there. If it is no to all of these, then do not catch it, and let another area do so.

FWIW, IMHO, your Get implementation is also overly complicated. I think typically, people would not expect a property do that sort of "work". My opinion only, though.

The only rule I can think of is something like "at the lowest level where you can actually do something useful with the information and you are not duplicating exception handling code".

For example, if you generally want to catch all database access related exceptions in the same way, I'd create a data abstraction layer (there are many good reasons to do so), and put the try-catch block there.

The other extreme would be a web app where you do not expect any such exceptions, and exceptions are caught by Elmah. In that case you wouldn't want to use a try-catch block at all because it would screw up logging.

It depends where you want to handle and to continue the processing. Suppose the calling code is in charge to notify the user, then a good place is the calling code. The calling code maybe need to ask the user something else after it has notified the user.

Also consider to reframe the exception which is thrown. If IsUsed resides in class which has nothing to do with databases and FindObject can throw a SqlServerTimedOutException, it will pop up the whole call stack. If this schemantics is utterly confusing catch the exception and rethrow it like this:

     public bool IsUsed  
     {   
         get 
         {    
          ClassA a = new ClassA();    
          Collection<ClassA> myCollection;

          try
          {
              myCollection = a.FindObject("Id = 1","");
          }
          catch (SqlServerTimedOutException ex)
          {
              throw new MyApplicationException("Cannot evaluate if is in use.", ex);
          }

          if(..) // etc   
         } 
    }

But don't overuse this, because it makes the code rather ugly. Consider carefully.

Generally, as a rule of thumb:

The try block contains the guarded code that may cause the exception

Internally in your case, you can handle it like:

FindObject()
{
try{}catch{throw;//or throw new Exception("Some info");
}

IsUser
{
try{...a.FindObject("Id = 1",""); return true;}
catch(Exception ex){Log(ex); return false;}
}

--EDIT--

This is in response to @controlbreak comment:

MSDN says:

You can explicitly throw an exception using the throw statement. You can also throw a caught exception again using the throw statement. It is good coding practice to add information to an exception that is re-thrown to provide more information when debugging.

As far I'm concerned, I put try catch blocks :

• when a finally is needed to cleanup ressources
• when a specific operation flow is needed in case of failure (retry by the user, logging...)

Generally, I let exceptions bubble up to the top level, usually some winform control event handler. I use to put try catch here, using an application level Exception handling method.

Otherwise, when I'm particularly lazy, I hook Application.ThreadException event to a MessageBox.Show in the static void Main() entry point method.

Catch errors as early as possible, and design the API call so it can handle failour without using exceptions. Uncaught exceptions are a menace for stability and five layers up you might not know if there are exceptions thrown in edge-cases, unless the lower API's express that they can fail.

Often you can design the api to have a result-object holding execution state in it, which will tell the consuming developer that this method can fail and how it fails.

Such an approach will often give you meaningfull way to handle exceptions in the lower api parts, instead of just letting them bubbel upwards to the calling code.

This said there are languages where you can state that a method will throw exceptions and which exceptions it throws, but you wrote C# which doesn't have this functionality.

See, if you are get any error set the IsUsed to Default. Something like this :

public bool IsUsed 
{  
 get
 {   
   try{
     ClassA a = new ClassA();   
     Collection<ClassA> myCollection = a.FindObject("Id = 1","");

     if(..) // etc  
   } 
   catch { return false;}
 } 
}

So whenever there is any error in FindObject you will produce false.

I disagree with most of you.

First of all I would like to recommend this great post by Joel: http://www.joelonsoftware.com/items/2003/10/13.html

Keeping that in mind, I use try-catch blocks only as close to WHERE they can go wrong as possible.
Obviously the UI would like to hear about it. Let it have an error value than, but don't hide the cause of the actual problem (by catching other accidental errors).

If I understand your code correctly, I think I would have used the try-catch on the actual query, and make IsUsed nullabe to make sure I understand it was not assigned a value.