8

I have some code that currently looks somewhat like this:

public void MainFunction() 
{
   try
   {
      SomeProblemFunction();
   }
   catch
   {
      AllFineFunction();
   }
}

private void SomeProblemFunction() { ... }
private void AllFineFunction() { ... }

As you can see, I'm currently wrapping the call to SomeProblemFunction around a try statement because that function could fail (it relies on an outside web service call).

My question is this: should the try statement be a) outside the problem function (like I have it now) or b) inside the problem function?

3
  • 3
    You should have the try in the function that can respond to exceptions. It's unclear solely on the information you've provided whether SomeProblemFunction should catch exceptions or not. But, for the record, just catch is not recommended. You should be catching only the exceptions you know you can handle. Feb 5, 2014 at 19:09
  • This other question will help you : stackoverflow.com/questions/14973642/…
    – Larry
    Feb 5, 2014 at 19:12
  • 1
    Here is the Herb Sutter's article which can clearly answer to your question: drdobbs.com/when-and-how-to-use-exceptions/184401836 Feb 5, 2014 at 19:22

6 Answers 6

7

Typically you want to allow your exceptions propagate up to your application boundaries. You're only going to want to do one of a few things with your exception:

  • Wrap it
  • Replace it
  • Let it propagate

Update

From your question it seems that you are looking for a fault tolerant solution for your web service calls. This is a more complex problem than simply "where do I put my try-catch?" You would still place your exception handling at the application boundary, but there you would implement your fault tolerance strategy. This would need to have many considerations, including asynchronously calling your web service, number of retry attempts, etc. I would suggest doing a search for web service fault tolerance.

2
  • What about retry logic? That could happen when the operation is attempted and failed, not back up in the application. Too narrow a recommendation, IMO. Feb 5, 2014 at 19:17
  • @PeterRitchie, I have updated my answer to include your concerns. Feb 5, 2014 at 19:29
4

What you have is correct; see the MSDN example:

public class ThrowTestB
{
    static void Main()
    {
        try
        {
            // TryCast produces an unhandled exception.
            TryCast();
        }
        catch (Exception ex)
        {
            // Catch the exception that is unhandled in TryCast.
            Console.WriteLine
                ("Catching the {0} exception triggers the finally block.",
                ex.GetType());

            // Restore the original unhandled exception. You might not 
            // know what exception to expect, or how to handle it, so pass  
            // it on. 
            throw;
        }
    }

    public static void TryCast()
    {
        int i = 123;
        string s = "Some string";
        object obj = s;

        try
        {
            // Invalid conversion; obj contains a string, not a numeric type.
            i = (int)obj;

            // The following statement is not run.
            Console.WriteLine("WriteLine at the end of the try block.");
        }
        finally
        {
            // Report that the finally block is run, and show that the value of 
            // i has not been changed.
            Console.WriteLine("\nIn the finally block in TryCast, i = {0}.\n", i);
        }
    }
    // Output: 
    // In the finally block in TryCast, i = 123. 

    // Catching the System.InvalidCastException exception triggers the finally block. 

    // Unhandled Exception: System.InvalidCastException: Specified cast is not valid.
}
2
  • 2
    While this link may answer the question, it is better to include the essential parts of the answer here and provide the link for reference. Link-only answers can become invalid if the linked page changes.
    – Raul Rene
    Feb 5, 2014 at 19:36
  • 1
    I concur with @RaulRene. I went ahead and edited it for you so it doesn't get deleted. Feb 5, 2014 at 19:50
2

As a rule of thumb I try and build code that focuses try catches to the exact spot the problem may occur.

That said both of your solutions are correct.

If it were my code I would do this

public void MainFunction() 
{
   try
   {
      SomeProblemFunction();
   }
   catch(Exception e)
   {
      Messagebox.Show(e.Message);
   }
}

private void SomeProblemFunction() {
    try{
        web call
    }
    catch{
         throw a specific exception related to this spot
    }
}
private void AllFineFunction() { ... }

With this method you can easily create applications that handle a slew of very accurate exceptions

4
  • How would you decide what exception to throw in SomeProblemFunction? You're not catching a particular exception? Feb 5, 2014 at 19:15
  • I left it blank because you can create your own or choose to catch what ever you want. I was just painting a picture Feb 5, 2014 at 19:32
  • The OP is already confused, it's apparent he needs some clear answers not more muddiness. Feb 5, 2014 at 19:35
  • 2
    @PeterRitchie well if you are correct luckly the OP has many other solutions to take. He is in no way obligated to take mine. Thank you for your insight Feb 5, 2014 at 19:43
2

A fine question, I think. I'll attempt an answer.


If you want to recover within SomeProblemFunction, then it would make perfect sense to move the try...catch inside of that method. If, however, you are comfortable saying that if anything fails in SomeProblemFunction, then the whole thing is a failure, then keep it as you have it now and recover in (or throw from) MainFunction.


Thanks to the comment below, I'm adding some clarity. Depending on the specific exception that is being thrown within SomeProblemFunction, you may not have the ability to recover within that method. If you have a mixture of recoverable and non-recoverable, then it would be prudent to have the try...catch in both places.

The most important thing is that you NEVER catch an exception from which you cannot recover without throwing it on after doing your thing. It's tempting to add big broad catches (catch (Exception)) to avoid your app crashing during development, but it is never worth it. If those things make it into your production code, you've introduced a problem-solving and debugging nightmare.

2
  • "Want" and "can" are two different things. Feb 5, 2014 at 19:16
  • Indeed, they are. I'll add a little clarity, thanks. Feb 5, 2014 at 19:20
1

In my opinion, there is no straight answer for this. The try catch is used to handle the exceptions that may occur. If your exception handling code is going in the main function then you should have the try catch in the main function. If your exception handling code in the problem function then you should add it to the problem function.

My preference though is to put it in both functions. If you put the try catch in the problem function, you can throw the exception and catch it in the main function. This always appear to other developers that the exception was thought of in that function and not missed handling it by mistake.

1

This depends on how severe this web service call failure would be.

Is it a failure that would prevent further processing of your code to run? If so, have no try/catch here, allowing it to propagate up to the guy who needs to know that this web service call failed. Optionally, you could still catch and throw a new exception with some more meaningful Exception type/ details.

Do you simply just want to re-try the web service call again if it doesn't work? IF so, then you have the try in the correct place; you just need to add a loop.

Is it not a big deal if this web service call fails? e.g. - will the rest of your code work OK? (I've found this to be uncommon). If so, leave the try/catch where it is, and log the error somewhere so that you're alerted.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.