Share via

Should I wrap these API calls in Try-Catch blocks? Best practice question.

  • Article

Question

Wednesday, January 4, 2012 12:57 PM

Hello folks,

I have created a C# wrapper DLL, which allows me to call iTextSharp functionality from my MFC application.

I do very little in the wrapper class.

Should I wrap each function call in a Try ... Catch block? And if yes, what is appropriate to do when an error is encountered? Just display an error message?

Should I have one Try-Catch for each class method? Or one Try-Catch for each API call?

My C# code looks like this...

 

 

/// <summary>

/// Draw an image into the location of the PDF document

 /// </summary>

public void DrawImageOnPdfDocumentFromArray(byte[] byteArrayIn, String strImageFieldName)

{

MemoryStream ms = new MemoryStream(byteArrayIn);

_PdfDocumentImage = Image.GetInstance(byteArrayIn);

PdfContentByte overContent = _OutputPdfStamper.GetOverContent(1);

//Specifying the name of the field where this image will be placed

 

float[] imageArea = _PdfFormFields.GetFieldPositions(strImageFieldName);

iTextSharp.text.

Rectangle imageRect = new Rectangle(imageArea[1], imageArea[2], imageArea[3], imageArea[4]);

_PdfDocumentImage.ScaleToFit(imageRect.Width, imageRect.Height);

_PdfDocumentImage.SetAbsolutePosition(imageArea[1] - _PdfDocumentImage.ScaledWidth + (imageRect.Width - _PdfDocumentImage.ScaledWidth) / 2, imageArea[2] + (imageRect.Height - _PdfDocumentImage.ScaledHeight) / 2);

overContent.AddImage(_PdfDocumentImage);

}

 

Thanks,

.......Cameron

Cameron Conacher

All replies (5)

Wednesday, January 4, 2012 6:10 PM âś…Answered | 2 votes

When asking yourself if you should catch an error or leave it be you need to first ask yourself these questions:

  • What will you do if you catch it?  Can you fix the problem, try again, skip this task, prompt the user for what to do (i.e. for bad user input tell them that they need to re-enter it, or maybe just tell them to try again later).
  • Logging errors is often appropriate and a good idea, but it's a separate issue.  Even if you log an error, you are still making a choice about whether you fix it, deal with it in some manor, or just ignore it (even if you log it).
  • Does it make sense to continue on with the program if you get an exception?  Is it a critical task that, if it fails, makes the rest of the method pointless?  For example, if you can't connect to a database there's no point in catching the error and then continuing on with your work using the bad connection.  You might as well just let the exception get thrown all the way up to a point where you are done with that connection (possibly to the UI to tell the user to try again later).
    On the other side of things, maybe you have some optional task and it's extremely important that even if there is a catastrophic error in methodA, the code in methodB after it MUST be run regardless of the success of method A.  This is a clear cut case where a try/catch should be wrapped around methodA.

In your case your question is should this method be wrapped in a try/catch.  So for this case, does whoever is going to be calling this method care if it was successful or not?  Does it need to know if there was a problem (i.e. does it need to notify someone that there was a failure)?  Will it be important that the code calling this method continue execution regardless of whether or not there was a problem?

A good example of the tradeoffs are the comparisons between int.Parse() and int.TryParse().  int.TryParse will never throw an exception.  It uses a return value to inform the caller of whether or not it was successful.  Parse on the other hand will throw an exception if there is a problem, and that is the only way that it informs the caller that it didn't work.  Now in this particular case (parsing an integer) if you are dealing with user input there is a reasonable expectation of problems, so you ought to take the time to check for a problem rather than using exceptions, so TryParse is better, but if you're dealing with information that you know is going to be an integer (i.e. something that was originally an integer but was serialized as a string for communication purposes) then int.Parse may be appropriate (and it will be shorter, faster, and easier by a very tiny margin).

Wednesday, January 4, 2012 1:10 PM

First of all you should avoid that an error could be throwed. 

But as we have to do with external connections that is impossible.

A Try and Catch is a last life line, not a kind of solution to fix everything which we don't understand.

In your sample for instance you can use first the IO.File.Exist("path") to see if it makes sense than creating a kind of try which can be the result of everything.

 

Success
Cor

Wednesday, January 4, 2012 2:37 PM

Thank you Cor.

I agree that a Try-Catch is not the first line of defense. I believe that they are expensive.

And, that is what I am trying to understand. I realize this is a very basic question. But since I am at the beginning, I want to adopt the best practices now, rather than trying to retrofit them at some later point.

Are you saying that all I really need to do is use IO.File.Exist, to determine whether or not my PDF is available. I don't need to worry about return values from any of the other API calls?

 

Thanks again for your time.

Cameron Conacher

Wednesday, January 4, 2012 6:33 PM | 2 votes

Cameron,

I gave you an obvious example in your code. And then the answer is yes, it makes no sense at all to start a procedure, let it hang a while and then throw an exception because there is no file available. So simply investigate that first.

However, that is just a sample. Every situation can be different, I've seen code like this in the forums (so bad like this code bellow I've never seen it, but whatever

 

       private Int64 Count(Int32 a, Int32 b)
        {
            Int64 c = 0;
            try
            {
               c= (Int64)(a + b);
               return c;
            }
            catch
            {
                return 0;
            }
            finally
            {
                if ((a + b) != c)
                {
                    throw new  SystemException();
                }
            }
        }

It gives the idea that somebody checks something, but it is useless and only good if you are paid per row of code.

 

 

Success
Cor

Wednesday, January 4, 2012 6:50 PM

Thanks Cor,

I try to write code so that if I get a call at two o'clock in the morning, and I am drunk, I have a good chance of understanding what I originally intended the code to do :-)

 

Cameron Conacher