General select statement to stored procedures

Posted on

Problem

I’m trying to write a general function to several specific stored procedures, but I’m afraid that it will cause errors that I don’t see now. The function is to get the name of the stored procedure as a string, and get a list of objects that contain the parameters of the stored procedure. Does it look good to you?

public static DataTable SelectQuery(string storedProcedureName, List<object> parameters)
    {
        DataTable dt = new DataTable();
        OleDbConnection con = WorkerDB.GetConnection();
        OleDbCommand cmd = new OleDbCommand();
        cmd.CommandText = storedProcedureName;  
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Connection = con;
        for (int i = 0; i < parameters.Count; i++)
        {
            cmd.Parameters.AddWithValue("?", parameters[i]);
        }
        try
        {
            con.Open();
            OleDbDataAdapter da = new OleDbDataAdapter(cmd);
            da.Fill(dt);
            da.Dispose();                
        }
        catch (Exception ex)
        {

            throw ex;
        }
        finally
        {
            con.Close();
        }

        return dt;
    }

Solution

This is first refactored version of your code. I have changed following thing

  1. If any object provide dispose method , prefer using statement.
  2. Use dictionary rather than list of parameter as you have a possibility of specifying param names also
  3. Current implementation sticks only to datatable , you might need to think about the datareader.
  4. naming of variable should be explicit.

Few more things needs to be thought like out param if you need.

public static DataTable ExecuteDataTable(string queryName, CommandType commandType, Dictionary<string,object> parameters=null)
{
    using (OleDbConnection connection =  WorkerDB.GetConnection())
    {
        using (var command=new OleDbCommand())
        {
            connection.Open();
            command.CommandText = queryName;
            command.CommandType = commandType;
            command.Connection = connection;


            if (parameters != null)
            {
                foreach (var parameter in parameters)
                {
                    command.Parameters.AddWithValue(parameter.Key, parameter.Value);
                } 
            }

            var dataAdapter = new OleDbDataAdapter(command);
            var dataTable = new DataTable();
            dataAdapter.Fill(dataTable);
            dataAdapter.Dispose();

            return dataTable;
        }
    }
}

Don’t do this:

catch (Exception ex)
{
    throw ex;
}

From MSDN

Once an exception is thrown, part of the information it carries is the
stack trace … If an exception is re-thrown by
specifying the exception in the throw statement, the stack trace is
restarted at the current method and the list of method calls between
the original method that threw the exception and the current method is
lost. To keep the original stack trace information with the exception,
use the throw statement without specifying the exception.

Leave a Reply

Your email address will not be published. Required fields are marked *