locked
Using, try/catch and disposing RRS feed

  • Question

  • User1428246847 posted

    I have a piece of code like below

    public static DataTable executeQuerySP(string strConnectionname, string strStoredProcedure, Hashtable htParameters, out string errmsg)
    {
        errmsg = "";
    
        if(string.IsNullOrEmpty(strConnectionname))
        {
            throw new ArgumentNullException("strConnectionstring");
        }
        if(String.IsNullOrEmpty(strStoredProcedure))
        {
            throw new ArgumentNullException("strStoredProcedure");
        }
    
        string connectionstring = getConnectionstring(strConnectionname, out errmsg);
        if (connectionstring == null)
            return null;
    
        DataTable tbl = new DataTable();
    
        try
        {
            using (SqlConnection conn = new SqlConnection(connectionstring))
            {
                using (SqlCommand cmd = new SqlCommand(strStoredProcedure, conn))
                {
                    // indicate it's an SP
                    cmd.CommandType = CommandType.StoredProcedure;
    
                    // add parameters to command
                    if (htParameters != null)
                    {
                        foreach (DictionaryEntry de in htParameters)
                        {
                            cmd.Parameters.AddWithValue(de.Key.ToString(), de.Value);
                        }
                    }
    
                    try
                    {
                        // open connection
                        conn.Open();
                        // execute
                        tbl.Load(cmd.ExecuteReader());
                    }
                    catch (SqlException sex)
                    {
                        //if (log.IsWarnEnabled) log.Warn(String.Format(Messages.fmtExceptionlog, System.Reflection.MethodBase.GetCurrentMethod().Name, sex.ToString()));
                        if (log.IsErrorEnabled) log.Error(String.Format(Messages.fmtExceptionlog, System.Reflection.MethodBase.GetCurrentMethod().Name, sex.Message));
                        errmsg = msgQueryError;
                        return null;
                    }
                } // using (SqlCommand cmd = new SqlCommand(strStoredProcedure))
            } // using (SqlConnection conn = new SqlConnection(connectionstring))
        }
        catch (Exception ex)
        {
            errmsg = String.Format(Messages.fmtExceptionlog, System.Reflection.MethodBase.GetCurrentMethod().Name, ex.ToString());
            if (log.IsErrorEnabled) log.Error(errmsg);
            errmsg = Messages.dicErrors["Exception"];
            return null;
        }
    
        return tbl;
    }

    There are two questions:

    1. If a SQL exception occurs, are the objects in the in the using blocks disposed?
    2. If another exception occurs, are the objects in the in the using blocks disposed?

    If I single step without forcing errors, the closing '}' of the using blocks are reached. When I force exceptions, they are not (for both cases). Can anybody shine some light on this?

    Friday, November 14, 2014 10:26 AM

Answers

  • User-40287846 posted

    Hi,

    Irrespective of error , the using will dispose the objects, and your code won't execute the 2nd try catch block, one main try catch block is enough to capture and return the error, once you get the error return block will execute and return  null, so the 2nd try catch won't execute , and instead of returning null, return some meaning full valid message to improve the performance .

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, November 14, 2014 10:45 AM
  • User753101303 posted

    Hi,

    Yes, as shown in the doc at http://msdn.microsoft.com/en-us/library/yh598w02.aspx ,"using" is just a convenient way to generate a try/finally block...

    If the main point is to log the error, using a global exception handler could perhaps be better. Returning null is not always a good idea as the code that follows is perhaps not prepared to handle this properly. As far as I know it's best to rather start at the highest possible level and go down as needed rather than to start by catching locally...

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, November 14, 2014 10:59 AM

All replies

  • User-40287846 posted

    Hi,

    Irrespective of error , the using will dispose the objects, and your code won't execute the 2nd try catch block, one main try catch block is enough to capture and return the error, once you get the error return block will execute and return  null, so the 2nd try catch won't execute , and instead of returning null, return some meaning full valid message to improve the performance .

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, November 14, 2014 10:45 AM
  • User753101303 posted

    Hi,

    Yes, as shown in the doc at http://msdn.microsoft.com/en-us/library/yh598w02.aspx ,"using" is just a convenient way to generate a try/finally block...

    If the main point is to log the error, using a global exception handler could perhaps be better. Returning null is not always a good idea as the code that follows is perhaps not prepared to handle this properly. As far as I know it's best to rather start at the highest possible level and go down as needed rather than to start by catching locally...

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, November 14, 2014 10:59 AM
  • User-434868552 posted

    @wim sturkenb...    TIMTOWTDI

    imho, it's better to use the strategy of methods like .TryParse whenever appropriate ... i do this for example when sending automated e-mails ... my methods for anything could go wrong situations usually return a Boolean and use out parameters for both the data that is required as well as any error text i wish to return.

    in your example, something like this:

    public static Boolean executeQuerySP(string strConnectionname, string strStoredProcedure,
    Hashtable htParameters,
    out DataTable sqlData, out List<String> errorMessages)

    if false, i use the error message(s) appropriately.  N.B.: as with the .TryParse methods, it is absolutely essential to inspect the returned Boolean.

    Since the error text may be cryptic for end users, often in my methods i will substitute more friendly to end user text in plain easy to understand language. 

    By using List<String> i can return multiple lines of text when necessary.

    Friday, November 14, 2014 1:18 PM
  • User1428246847 posted

    @sen338

    Thanks for the reply. The second catch will be executed if the exception was not an SQL exception and covers possible other errors; it might be better to have the outer try / catch outside the method and around the caller (or elsewhere). But at this stage I prefer the way shown as I feel that it keeps the code slightly more compact.

    Question: how would I return a useful error message in a datatable (or in an int if I run a ExecuteNonQuery())? Currently I return a user-friendly error in the parameter errmsg.

    Friday, November 14, 2014 1:26 PM
  • User1428246847 posted

    @PatriceSc

    Thanks for the reply. The main point is to be able to return different user-friendly messages depending on the exception thrown while keeping similar logging. Although the example did not show it, there will be two try/catch blocks; one for the opening of the connection (and returning a user-friendly 'database connection error' message) and one for the execution of the stored procedure (a user-friendly 'error retrieving data' message in this case).

    Regarding returning null, I don't see a problem. My code will handle it and interpret it as an error.

    As indicated in my reply to sen338, where one catches exceptions is, in my opinion, a matter of preference. But if you can point me to some interesting articles, I might change my mind ;-)

    Friday, November 14, 2014 2:42 PM
  • User1428246847 posted

    @gerrylowry

    Thanks for the reply. Using 'out' might be a matter of preference / style; it might make a more consistent code though so I will chew on that for a while. I like the idea of being able to return multiple error messages in a list; I usually bail out on the first error, but I can see some uses (at this stage I concatenate message if necessary).

    Friday, November 14, 2014 2:58 PM