Commenting try catch statements

Question

What is the proper place to explain error handling in a try-catch statement? It seems like you could put explanatory comments at either the beginning of the try block or the catch block.

// Possible comment location 1
try
{   
    // real code
}
// Possible comment location 2
catch
{
    // Possible comment location 3

    // Error handling code

}

Answer 1

I usually do the following. If there's only one exception being handled, I usually don't bother since it should be self-documenting.

try
{   
    real code // throws SomeException
    real code // throws SomeOtherException
}
catch(SomeException se)
{
    // explain your error handling choice if it's not obvious
}
catch(SomeOtherException soe)
{
    // explain your error handling choice if it's not obvious
}

Answer 2

"A comment is a lie". Work on those variable names and the general logic so you can avoid it. And if you really need to lie, do it inside the catch block.

Answer 3

I don't think it matters, at all.

I think the import thing to remember with commenting is to address why the code is the way it is and not what the code is doing, first and foremost. This is not to say you shouldn't explain complex logic in a concise comment, but the why is so much more important.

Answer 4

What about just setting up the code so it doesn't need extra comments?

try
{ 
   performDifficultAct( parameter );
}
catch (ArgumentOutOfRangeException couldNotFindArgument)
{
   // handle exception
}
catch (Exception otherUnknownException )
{
   // handle exception
}

No need to document if you can use your variable and method naming to show what is going on. No need to document if you are having to log or raise the exceptions - the logging message in the source code should be self-explanatory anyway. The only time you should need extra documentation in your code is when it is totally non-obvious what the code is doing or ther is an easy-to-miss gotcha or ambiguous step you have to add that will need explanation for anyone looking at the code in future.

Edit: To clarify a little, here's a bit more of how I might use those "catch" statements to provide useful information both to a maintenance programmer and to users/support/QA/anyone else who uses the software. Also an illustration of the kind of situation where I absolutely would want to add extra comments in the code:

public void PerformSomeActionOrOther(string parameter)
{
  try
  { 
     // For some reason an eleven character string causes a bluescreen from Kernel32
     if (parameter.Length==11) parameter+=" ";

     performDifficultAct( parameter );
  }
  catch (ArgumentOutOfRangeException couldNotFindArgument)
  {
     this.Log.WriteLn("Argument out of range exception in ArbitraryClass.PerformSomeActionOrOther");
     this.Log.WriteLn(String.Format("Probable cause is that {0} is not in the array", parameter));
     this.Log.WriteLn(String.Format("Exception: {0}", couldNotFindArgument.Message));
  }
  catch (Exception otherUnknownException )
  {
     this.Log.WriteLn("Unexpected exception in ArbitraryClass.PerformSomeActionOrOther");
     this.Log.WriteLn(String.Format("Exception: {0}", otherUnknownException.Message));
     throw( otherUnknownException );
  }
}

Answer 5

Definitely don't comment the top of it, because what can you usefully say except "starting an exception handling block here"? Comments on the catch statements are better, but in general, again, what are you gonna say? "Handle a NullPointerException"?

I'd go for a comment IFF you need to say that you're doing something exciting, like chaining to an application-domain exception.

Answer 6

I think a well written try/catch should be concise and specific. I agree with @Jason that the why is more important but equally, it is important to keep the code inside catch as concise as possible.

It would also help if you used specific exceptions to be caught. If you are using Java for example, try to catch a NullPointerException rather than a generic Exception. This should explain why the try catch exists and what you are doing to resolve it.

Answer 7

The location does not matter as long as you are consistent. My personal preference is as follows:

//comment 1: code does XYZ, can cause exceptions A, B, C
try {
    //do something
}
//comment 2: exception A occurs when foo != bar
catch (ExceptionA a) {
    //do something
}
//comment 3: exception B occurs when bar is null
catch (ExceptionB b) {
    //do something
}
//comment 4: exception B occurs when foo is null
catch (ExceptionC c) {
    //do something
}

Answer 8

I know this isn't the answer you're looking for, but don't comment at all. If your code isn't clear enough to stand on its own without commenting, then you should refactor it until it is. Jeffrey Palermo just wrote a blog post that states it best.

Typically, comments tend to document either:

• Code that's too compact. Things that look like this: ++i?--g:h-i;
• Long blocks of code that need to be summarized
• Code that is either disposable or has no clear reason for existing

See below for a simplified example of some simple commenting on your exception block, and a version that eliminates the need for comments.

bool retries = 0;
while (retries < MAX_RETRIES)
{
    try
    {
        ... database access code
        break;
    }
    // If under max retries, log and increment, otherwise rethrow
    catch (SqlException e)
    {
        logger.LogWarning(e);
        if (++retries >= MAX_RETRIES)
        {
            throw new MaxRetriesException(MAX_RETRIES, e);
        }
    }
    // Can't retry.  Log error and rethrow.
    catch (ApplicationException e)
    {
        logger.LogError(e);
        throw;
    }
}

While the above comments promote reusability, you essentially have to maintain both the code and the comments. It is possible (and preferable) to refactor this so that it is clearer without comments.

bool retries = 0;
while (canRetry(retries))
{
    try
    {
        ... database access code
        break;
    }
    catch (SqlException e)
    {
        logger.LogWarning(e);
        retries = incrementRetriesOrThrowIfMaxReached(retries, e);
    }
    catch (ApplicationException e)
    {
        logger.LogError(e);
        throw;
    }
}

...

private void incrementRetriesOrThrowIfMaxReached(int retries, Exception e)
{
    if (++retries >= MAX_RETRIES)
        throw new MaxRetriesException(MAX_RETRIES, e);

    return retries;
}

private bool canRetry(int retries)
{
    return retries < MAX_RETRIES;
}

The latter example may seem like more code for a very subtle benefit, but the gains can't be overstated. The code is just as understandable, but you have the benefit that you don't need to have a separate set of metadata (comments) in order to explain the code. The code explains itself. If your catch code block is too long and needs comment to summarize, think about refactoring it to a separate method in order to improve readability.