1

Thanks to some tips and reminders here, I changed my code from this kludgy mess:

try
{
    DataSet dsUsage = new DataSet();

    SqlConnection conn = new SqlConnection("SERVER=PROSQL05;DATABASE=platypusdata;UID=duckbill;PWD=poisonToe42;Connection Timeout=0");

    SqlDataAdapter da = new SqlDataAdapter();

    SqlCommand cmd = conn.CreateCommand();
    cmd.CommandText = String.Format("Exec sp_ViewProductUsage_MappingRS '{0}', '{1}', '{2}'", mammal, dateBegin, dateEnd);
    da.SelectCommand = cmd;

    conn.Open();
    da.Fill(dsUsage);
    conn.Close();

    DataTable dtUsage = dsUsage.Tables[0];

    if (dtUsage.Rows.Count > 0)
    {
        foreach (DataRow productUsageByMonthDataRow in dtUsage.Rows)
        {
            . . .

...to this:

try
{
    SqlDataAdapter da = new SqlDataAdapter();
    DataSet dsUsage = new DataSet();

    using (SqlConnection conn = new SqlConnection(UsageRptConstsAndUtils.PlatypusConnStr))
    {
        using (SqlCommand cmd = new SqlCommand("sp_ViewProductUsage_MappingRS", conn))
        {
            cmd.CommandType = CommandType.StoredProcedure;

            cmd.Parameters.Add("@Unit", SqlDbType.VarChar).Value = _unit;
            cmd.Parameters.Add("@BegDate", SqlDbType.DateTime).Value = dtBegin;
            cmd.Parameters.Add("@EndDate", SqlDbType.DateTime).Value = dtEnd;

            da.SelectCommand = cmd;

            conn.Open();
            //cmd.ExecuteReader(); <- Is this even necessary?
            da.Fill(dsUsage);
        }
    }

    DataTable dtUsage = dsUsage.Tables[0];

    if (dtUsage.Rows.Count > 0)
    {
        // Populate the cells
        foreach (DataRow productUsageByMonthDataRow in dtUsage.Rows)
        {
            . . .

Note that I have SqlCommand's ExecuteReader commented out in the new code because it seems unnecessary due to the SqlDataAdapter being provided the SqlCommand. It works fine. So: am I correct in assuming I can remove cmd.ExecuteReader() altogether? Is there any benefit in retaining it, or would that be totally redundant and create "busy work" for the process?

UPDATE

So, to pass an array of SqlParameter (to the ExecuteDataSet method in MethodMan's answer), I take it that I would first have to do something like:

SqlParameter sqlp = new SqlParameter();
sqlp.ParameterName = "Unit";
sqlp.Value = _unit;
cmd.Parameters.Add(sqlp);

...etc. (and then add them to an array - or, possibly better a generic list of SqlParameter).

UPDATE 2

I just ran into this for the first time: if you use MethodMan's example (which I do) and you use a parameterless query, you need to bypass the parameter-adding loop like so:

if (null != parameters)
{
    foreach (var item in parameters)
    {
        cmd.Parameters.Add(item);
    }
}
Community
  • 1
  • 1
B. Clay Shannon-B. Crow Raven
  • 8,547
  • 144
  • 472
  • 862
  • 1
    you are using the `DataSet.Fill()` method so you kind of answered your own question.. don't you think..? also unless you are wanting to loop through the results in a `While Loop` then what you are doing is fine also Iwould personally change the `cmd.Add` method to utilize cmd.Parameters.AddWithValue` and let the sever resolve / handle the datatype – MethodMan Nov 23 '15 at 17:40
  • 3
    It would be necessary if you are about to remove the dataset/datatable/dataadapter and use the datareader directly. Based on what little code you've posted, it appears you don't actually need the dataset at all and would be better off just using the datareader. – Robert McKee Nov 23 '15 at 17:44
  • 1
    @B.ClayShannon it's not necessary as I have stated earlier, to have the `cmd.ExecuteReader()` – MethodMan Nov 23 '15 at 18:25

1 Answers1

1

I would personally create a SqlDBHelper class and pass call the stored procedure using a method such as this

public static class SqlDBHelper
{
    public static DataSet ExecuteDataSet(string sql, CommandType cmdType, params SqlParameter[] parameters)
    {
        using (DataSet ds = new DataSet())
        using (SqlConnection connStr = new SqlConnection(ConfigurationManager.ConnectionStrings["DbConn"].ConnectionString))
        using (SqlCommand cmd = new SqlCommand(sql, connStr))
        {
            cmd.CommandType = cmdType;
            foreach (var item in parameters)
            {
                cmd.Parameters.Add(item);
            }

            try
            {
                cmd.Connection.Open();
                new SqlDataAdapter(cmd).Fill(ds);
            }
            catch (SqlException ex)
            {
                //log to a file or write to Console for example
                Console.WriteLine(ex.Message);
            }
            return ds;
        }
    }
}

If you want to return a DataTable then change the return type in the Method signature and call the following in the return statement below

public static DataTable ExecuteDataSet(string sql, CommandType cmdType, params SqlParameter[] parameters)

return ds.Tables[0];

Here is an example on how you would call the method

someDataTable = SqlDBHelper.ExecuteDataSet("sp_ViewProductUsage_MappingRS", CommandType.StoredProcedure,
            new SqlParameter() { ParameterName = "@Unit", SqlDbType = SqlDbType.VarChar, Value = _unit },
            new SqlParameter() { ParameterName = "@BegDate", SqlDbType = SqlDbType.DateTime, Value = dtBegin },
            new SqlParameter() { ParameterName = "@EndDate", SqlDbType = SqlDbType.DateTime, Value = dtEnd }
            );
MethodMan
  • 18,625
  • 6
  • 34
  • 52
  • Does the array of SqlParameter contain the values to be assigned the parameters, too? – B. Clay Shannon-B. Crow Raven Nov 23 '15 at 18:06
  • 1
    The Parameters are declared on the Sql Server side using the `@` param name the values are passed in order from outside the method call it's pretty simple to use so basically you would call `var someDataTable = ExecuteDataSet("sp_name", cmdType.StoredProc, parameters[] in order` does this make since..? – MethodMan Nov 23 '15 at 18:10
  • @canon not it's not an answer to a different question its an answer on how to better do what he's trying to do without all the redundancy .. – MethodMan Nov 23 '15 at 18:16
  • @MethodMan: Most of it makes sense, but what I'm unsure about is what exactly is contained in the parameters array when it is passed. Your code showseach parameter being added to the sqlCommand's Parameters collection, but what I wonder about is just how that array of parameters is constructed on the caller's side. It obviously has to contain both the Parameter name (such as "Unit") and the Parameter value (such as "YogiBear"). – B. Clay Shannon-B. Crow Raven Nov 23 '15 at 18:17
  • I did answer the question in my initial comment I have given @B.ClayShannon an example on how to execute the call – MethodMan Nov 23 '15 at 18:24
  • @canon I think that you are missing the point why would the OP want to make numerous calls to return data when all they need to do is create a method that's can be called from many different places in code using a static method that does the majority of the leg work for them..? either way I believe that the OP get the gist of what I have shown them. – MethodMan Nov 23 '15 at 19:26