Test connection before get data with data reader

Question

I have come up with the following "pattern" to get data from a database. For sure, I wanted to check if connection could be established, so I need to use a try-catch block. It works fine but I am curious if there is a better/faster/safer way.

    public List<T> GetAll(string sql)
    {
        var list = new List<T>();
        var db = new SqlDatabase(Registry.Instance.CurrentConnectionString);
        var cmd = new SqlCommand(sql);

        IDataReader reader = null;

        using (cmd)
        {
            try
            {                    
                reader = db.ExecuteReader(cmd);
            }
            catch (SqlException ex)
            {
                ShowExceptionError(ex);
                return list;
            }
        }

        using (reader)
        {
            while (reader.Read())
                list.Add(GetMethod(reader));

            reader.Close();
        }

        return list;
    }

I know that the best way is to declare the dataReader in the using block, but since I need the reference outside the try-catch block and I don't refer to it outside the using block, I think it's fine.

Answer 1

This is purely subjective, but I'd suggest instead of using the "using" blocks, use the "try/catch/finally" block. Something like this:

public List<T> GetAll(string sql)
{
    var list = new List<T>();

    SqlDatabase db = null;
    SqlCommand cmd = null;
    IDataReader reader = null;

    try
    {                    
        db = new SqlDatabase(Registry.Instance.CurrentConnectionString);
        cmd = new SqlCommand(sql);

        reader = db.ExecuteReader(cmd);

        while (reader.Read())
        {
            list.Add(GetMethod(reader));
        }
    }
    catch (SqlException ex)
    {
        // Deal with sql exceptions
        ShowExceptionError(ex);
        return list;
    }
    catch (Exception ex)
    {
        // Deal with all other exceptions
        ShowExceptionError(ex);
        return list;
    }
    finally
    {
        if (db != null)
        {
            db.Dispose();
        }

        if (cmd != null)
        {
            cmd.Dispose();
        }

        if (reader != null)
        {
            reader.Dispose();
        }
    }

    return list;
}

The idea here is to initialize everything inside the try block, so that if anything fails, one of the catch statements will handle it. I always use an additional catch(Exception e) block which handles anything that I didn't predict. The finally block will happen in both cases whether the code successfully executes or if there's an exception.

Also, there should be no need for closing the reader because the Dispose method will handle it. The code above is not tested and some parts might fail. The problem in question might be if one of the objects does not implement IDisposable interface, you can't use it in the using block and you can't dispose them. In that case just remove it from the finally block.