Central Database class I have this web site I inherited as part of my job and it has some old code.  I'd like to optimize it, but I don't really have the time to do it right, so I'm looking for some "low hanging fruit" to optimize. Basically I want minimal change with the greatest performance improvement.

One of the things I've come across is an old static method used to grab data from a remote database:

public static IDataReader GetRS(string sql, string connString)
{
    SqlConnection dbconn = new SqlConnection();
    if (connString.ToLower().Trim() == "shop")
        dbconn.ConnectionString = ConfigurationManager.ConnectionStrings["Shop.DbConnection"].ConnectionString;
    else
        dbconn.ConnectionString = connString;
    dbconn.Open();
    SqlCommand cmd = new SqlCommand(sql, dbconn);
    return cmd.ExecuteReader(CommandBehavior.CloseConnection);
}


I think this code is contributing to a high number of NumberOfReclaimedConnections we are seeing on the servers due to the calling code not cleaning up properly.  If I rewrite the above method like the one below, will it help make things more efficient without having to rewrite all the hundreds of instances of calling code?

public static IDataReader GetRS(string sql, string connString)
{
    IDataReader ans = null;
    using (DataTable tbl = new DataTable())
    {
        using (SqlConnection dbconn = new SqlConnection())
        {
            if (connString.ToLower().Trim() == "shop")
                dbconn.ConnectionString = ConfigurationManager.ConnectionStrings["Shop.DbConnection"].ConnectionString;
            else
                dbconn.ConnectionString = connString;
            using (SqlCommand cmd = new SqlCommand(sql, dbconn))
            {
                using (SqlDataAdapter adapter = new SqlDataAdapter(cmd))
                { adapter.Fill(tbl); }
            }
        }
        if (tbl.Rows.Count &gt; 0)
        {
            ans = tbl.CreateDataReader();
        }
    }
    return ans;
}


I'm pretty sure this will do a better job at cleaning up the connection as it will be disposed before the method returns and even if there is an error it will be disposed. Does the DataTable being disposed interfere with the caller's ability to extract data from the DataTableReader?  If I do need to remove the using around the DataTable, will not cleaning up the DataTable cause similar issues to not cleaning up the SqlConnection as it is now?

I'm also open to writing this code in a completely different way as long as I'm fulfilling the following:


I don't have to rewrite code on all the calling references to this method, I just don't have time to do that right now. But it's on the list.
Due to the above requirement, the return signature has to be compatible with an IDataReader. I'm open to writing a custom class that inherits this interface if that is what helps.
The code targets and will compile in a .NET 2.0 project. We're behind the technology curve.
I would like to point some opportunities here. As you are refactoring your old code, I suggest:


try to avoid the use of literal string inside methods. Instead, use const variables or Resources (I do prefer Resources);
create your variables where you need them. Move your using to the nearest point of interest;
try to use string.Equals, it will be faster; 
get your DbCommand directly from your connection, you have a method to do this;
try to use a Factory Pattern in this class, so in the future, if you need to change your database, you won't have to search for all your SqlConnection objects, you'll just need to change your Factory creator;
don't forget your XML comments


I made an example just to show those points. Sorry for the simplicity, but it's more difficult without knowing all your solution, but here is the idea:

using System;
using System.Configuration;
using System.Data;
using System.Data.Common;

namespace ClassLibrary1
{
    public class Class1
    {
        public const string shopStr = "shop";
        public const string shopDbConn = "Shop.DbConnection";
        public const string factoryDb = "System.Data.SqlClient";

        private static DbProviderFactory factory = DbProviderFactories.GetFactory(factoryDb);

        /// &lt;summary&gt;
        /// Get data from the database.
        /// &lt;/summary&gt;
        /// &lt;param name="sql"&gt;SQL command to retrieve data.&lt;/param&gt;
        /// &lt;param name="connString"&gt;Connection string for the database.&lt;/param&gt;
        /// &lt;returns&gt;&lt;c&gt;IDataReader&lt;/c&gt; with the first result set, or &lt;c&gt;null&lt;/c&gt; if no result was found.&lt;/returns&gt;
        public static IDataReader GetRS(string sql, string connString)
        {
            IDataReader result = null;

            using (DbConnection conn = factory.CreateConnection())
            {
                if (string.Equals(connString, shopStr, StringComparison.OrdinalIgnoreCase))
                    conn.ConnectionString = ConfigurationManager.ConnectionStrings[shopDbConn].ConnectionString;
                else
                    conn.ConnectionString = connString;

                using (DbCommand cmd = conn.CreateCommand())
                {
                    cmd.CommandText = sql;

                    using (DbDataAdapter adapter = factory.CreateDataAdapter())
                    {
                        DataTable tbl = new DataTable(); // don't dispose it
                        adapter.Fill(tbl);
                        if (tbl.Rows.Count &gt; 0) result = tbl.CreateDataReader();
                    }
                }
            }

            return result;
        }
    }
}