locked
How to handle errors properly RRS feed

  • Question

  • Hi everybody,

    We programmed several overloads of the ExecuteSqlCommand method. In all of these overloads we are passing messageText and statusCode parameters by reference. The statusCode is normally 0, but in case of error we set it to 400 (or some other number). We also format the return message (in case of error we set it to the exception message or format special way). In addition, in case we want to return a scalar value, we return that value in the messageText also (and then we convert to correct type in the calling method).

    The method itself returns true if there are no errors and false if there was an exception.

    I am thinking and proposing to our team to remove parameters passed by reference and set the StatusCode and MessageText in the class properties instead. Is it better than our original code or there is yet better solution?

    I will appreciate some ideas of how to handle this the most efficient and good way.


    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog


    • Edited by Naomi N Thursday, February 21, 2013 2:01 AM
    Wednesday, February 20, 2013 11:18 PM

Answers

  • Hi,

    I'm not a big fan of turning back exceptions into return codes (it makes ignoring them quite easy and then you know you had an exception if you read the log file, how much frequently is it checked ? Have you checked the code to see if the status code you get is always handled properly or sometimes just ignored ?).

    My personal preference is to handle exceptions at the *highest* possible level (if you catch at the lowest level, then that's it, the exception is just hidden from higher levels and there is nothing you can do). You can have a global handler that will act as a "safety net" for those who are not handled locally and that you can use to report and log errors...

    At a local level use try/finally or better a using statement to just release unmanaged resource (the exception still flows upward). Have a local catch clause (and using the more specific type possible) only if you better know than the global exception handler what should be done for this particular exception.


    Please always mark whatever response solved your issue so that the thread is properly marked as "Answered".

    • Marked as answer by Bob Shen Tuesday, March 5, 2013 2:48 AM
    Thursday, February 21, 2013 7:19 PM
  • I do like creating exception classes that contain all the information needed to debug errors.  But I don't like and don't practice (OUT and REF) constructs (personal habit).

    On the other hand, you could ask them why not log the error to a database and pass back the UNIQUE ID.  This has a big advantage of being process independant.  In addition it's easy to write program exception monitors too! 

    I'm not sure how SCOM internals work but it has to be SQL driven doesn't it?


    JP Cowboy Coders Unite!


    • Marked as answer by Bob Shen Tuesday, March 5, 2013 2:49 AM
    • Edited by Mr. Javaman II Tuesday, April 23, 2013 6:13 PM
    Thursday, February 21, 2013 7:47 PM

All replies

  • Do you have sample code to demo of what you are explaining? That will be easy for the rest to understand I guess.

    chanmm


    chanmm

    Thursday, February 21, 2013 6:57 AM
  • I am thinking and proposing to our team to remove parameters passed by reference and set the StatusCode and MessageText in the class properties instead. Is it better than our original code or there is yet better solution?

    I personally prefer setting the properties which is clearer. And you can pass only the statuscode and get the corresponding error message based on the error code. This way, it'd be easier and more scalable.

    Catherine Sea
    SourceSafe Replacement | Dynamsoft TFS Hosting

    Thursday, February 21, 2013 7:14 AM
  • I've been thinking lately that we may want to introduce a return object (could be a new class).

    Say, this is one of our current overloads

    internal Boolean ExecuteSqlCommand(SqlCommand sqlCommand, ReturnType returnType, ref Int32 recordCount, ref String returnMessage, ref Int32 statusCode)
            {
                // For STR, XML, and XM2 return types
                // Execute the query and populate the formatted return string
                returnMessage = "";
                recordCount = 0;
    
                sqlCommand.Connection = sqlConnection;
    
                SqlDataReader sqlDataReader = null;
                try
                {
                    // Execute the command
                    sqlDataReader = sqlCommand.ExecuteReader();
    
                    // Generate the return string (STR/XML/XM2), if applicable
                    returnMessage = GetFormattedReturn(sqlDataReader, returnType, out recordCount);
                    if (0 == recordCount && ReturnType.STR == returnType && String.IsNullOrWhiteSpace(returnMessage))
                    {
                        returnMessage = "DONE";
                        statusCode = 1;
                    }
    
                    // NN 02/05/2013 Added support for multiple result sets
                    if (0 == recordCount && ReturnType.STR == returnType)
                    {
                        // Do nothing
                    }
                    else
                    {
                        StringBuilder result = new StringBuilder(returnMessage);
    
                        while (sqlDataReader.NextResult())
                        {
                            result.AppendFormat("\r\n{0}", GetFormattedReturn(sqlDataReader, returnType, out recordCount));
                        }
                        returnMessage = result.ToString();
                    }
    
                    // Check for uncommitted transactions and commit them, if necessary
                    String command = sqlCommand.CommandText;
                    CheckUncommittedTransactions(sqlCommand);
                }
                catch (Exception ex)
                {
                    returnMessage = ex.ToString();
                    Logging.Log(returnMessage, 1);
                    statusCode = 400;
                    return false;
                }
                finally
                {
                    if (null != sqlDataReader && !sqlDataReader.IsClosed)
                    {
                        sqlDataReader.Close();
    //                    sqlDataReader.Dispose();
                    }
                }
                return true;
            }

    What is the proper way of writing such a method that gets a result set in a reader, formats it and returns formatted output back?

    Thanks in advance.


    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog

    Thursday, February 21, 2013 1:23 PM
  • So, I didn't win so far. My colleague thinks that OUT parameters have advantage of making sure we set them (although right now we're passing these parameters by reference). In other words, we decided to keep everything as is for now and continue development.

    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog

    Thursday, February 21, 2013 6:34 PM
  • Hi,

    I'm not a big fan of turning back exceptions into return codes (it makes ignoring them quite easy and then you know you had an exception if you read the log file, how much frequently is it checked ? Have you checked the code to see if the status code you get is always handled properly or sometimes just ignored ?).

    My personal preference is to handle exceptions at the *highest* possible level (if you catch at the lowest level, then that's it, the exception is just hidden from higher levels and there is nothing you can do). You can have a global handler that will act as a "safety net" for those who are not handled locally and that you can use to report and log errors...

    At a local level use try/finally or better a using statement to just release unmanaged resource (the exception still flows upward). Have a local catch clause (and using the more specific type possible) only if you better know than the global exception handler what should be done for this particular exception.


    Please always mark whatever response solved your issue so that the thread is properly marked as "Answered".

    • Marked as answer by Bob Shen Tuesday, March 5, 2013 2:48 AM
    Thursday, February 21, 2013 7:19 PM
  • I do like creating exception classes that contain all the information needed to debug errors.  But I don't like and don't practice (OUT and REF) constructs (personal habit).

    On the other hand, you could ask them why not log the error to a database and pass back the UNIQUE ID.  This has a big advantage of being process independant.  In addition it's easy to write program exception monitors too! 

    I'm not sure how SCOM internals work but it has to be SQL driven doesn't it?


    JP Cowboy Coders Unite!


    • Marked as answer by Bob Shen Tuesday, March 5, 2013 2:49 AM
    • Edited by Mr. Javaman II Tuesday, April 23, 2013 6:13 PM
    Thursday, February 21, 2013 7:47 PM
  • Do you have a reference to the other thread about Exception Handling which was few weeks after this one?

    Thanks in advance.


    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog

    Monday, April 22, 2013 6:16 PM
  • BTW, looks like my colleague did implement a custom Exception class, so this discussion will become relevant again.

    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog

    Monday, April 22, 2013 6:18 PM
  •  Ok good we look forward to futher discussion.

    JP Cowboy Coders Unite!

    Tuesday, April 23, 2013 6:16 PM