2
\$\begingroup\$

I am interested in finding out what is the correct way of implementing error handling for this situation in C#.

The system tries to do an operation. If the operation succeeded (returns a non-error code), the system logs in a log database that the operation was succesfull. If the operation returns a error code or something caused an exception before the operation, the system will log an error. However, if the an error is caught when logging, the system shouldn't log an error in the database.

I am using C#. The following code is what I have used until now, but I don't know if it is the best practice in this situation.

int response = -1;  //some error code
try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions
}
catch
{
    //error handling
}

try
{
    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch
{
    //logging error handling
}

Do you have any suggestions for improving my code?

Note: The catch blocks include in the original code handling specific errors, I just used a general catch blocks for code simplicity in my question.

\$\endgroup\$

closed as off-topic by 200_success Apr 5 '16 at 21:55

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

3
\$\begingroup\$

You could combine the try-catch blocks:

int response = -1;  //some error code
var logging = false;
try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions

    logging = true;
    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch
{
    if(logging)
    //logging error handling
    else
    //error handling
}

Even better, if you know the logging could throw a particular type of exception that the other operations will not, then catch just that exception and handle it as a logging exception, and let all others trickle down to a general catch:

int response = -1;  //some error code

try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions

    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch(LoggingSpecificException)
{
    //logging error handling
}
catch
{
    //error handling
}

As far as the general strategy, it doesn't look too bad. Getting a "return status code" from a method is more than a little outdated (try-throw-catch was designed to replace this style of method return), but there's a lot of "legacy code" out there, and some built-in methods still return values that indicate failure. The return value of DoOperation(), if any, should be its conceptual "product"; data produced by DoOperation as the result of a computation. If DoOperation, conceptually, doesn't make any such calculation, it would be better conceptually (and thus from an understandability standpoint) to return void and instead throw exceptions on errors. However, you'd have to throw and catch multiple types of exceptions, or keep track of multiple states instead of just "logging".

Also, I don't know if you simply omitted it for the operation, but I would think that knowledge of exactly what went wrong might be good to know, and so any general catch should be catch(Exception ex) or similar to allow use of exception data when handling the error.

\$\endgroup\$
  • \$\begingroup\$ The DoOperation() is legacy code. The problem is that there isn't just one DoOperation(); it's just an generalization for many operations done in a system. In some cases, the return code reflects if the operation could be made or not. But in others the DoOperation returns some codes for partial success; for example some actions failed but others before them not. The actions that failed are sufficiently unimportant to not cause the need of rollback. \$\endgroup\$ – Coral Doe Jul 26 '12 at 15:10
  • 1
    \$\begingroup\$ "it's just an generalization for many operations done in a system" maybe Chain of Responsibility pattern may help? \$\endgroup\$ – Codium Jul 26 '12 at 16:32
  • \$\begingroup\$ Thanks for the info. I checked chain of responsability pattern and it's just what I needed to improve other parts of the code. \$\endgroup\$ – Coral Doe Jul 27 '12 at 14:36

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