repito3000
repito3000

Reputation: 5

which dr is open?(System.InvalidOperationException: 'There is already an open DataReader associated with this Command which must be closed first.')

i am getting the followed exeption(System.InvalidOperationException: 'There is already an open DataReader associated with this Command which must be closed first.') and i cant figure out how to fix it. I would apreciate some help

    private void button2_Click(object sender, EventArgs e) // PLACING ORDERS
    {
        int qty1 = 0;
        int cmd2_num;
        SqlConnection con = new SqlConnection("Data Source=.\\sqlexpress;Initial Catalog=adventureworks2012;" + "User ID=sarr**strong *strong text*text**;Password=1234");
        con.Open();
        
        SqlCommand cmd1 = new SqlCommand ("select Quantity from Production.ProductInventory where productid ='" + int.Parse(textBox1.Text) + "'",con);
        SqlCommand cmd2 = new SqlCommand(
            "'UPDATE Production.ProductInventory" +
            "SET Quantity -='" + qty +
            "'WHERE ProductId = '" + textBox1.Text + "'",con);

        SqlDataReader dr = cmd1.ExecuteReader();
        
        
        while (dr.Read())
        {
            qty1 = int.Parse(dr.GetValue(0).ToString());
        }
        if (qty1 >= qty)
        {
            cmd2_num = cmd2.ExecuteNonQuery();
        }
        else
        {
            MessageBox.Show("unfortunatly we are not able to provide you with the amount you want", "Oops!", MessageBoxButtons.OK, MessageBoxIcon.Warning);
        }

        con.Close(); 
    }

Upvotes: 0

Views: 64

Answers (1)

Charlieface
Charlieface

Reputation: 71544

You have a number of issues with your code:

  • Your primary issue: not disposing the reader object from the first command
  • You are also blocking the thread with a message box while the connection is still open, for the same reason
  • You are interpolating user input straight into the query, this leaves you vulnerable to injection attacks and syntax errors. A nice way to ensure you don't do this is to store the batch in a const string
  • Your update query also has extra ' quotes which are syntax errors
  • productid is not a string, in which case you should not enclose it in quotes anyway
  • If you get an int result from a reader, you shouldn't stringify it and then parse it again int.Parse(dr.GetValue(0).ToString()) just cast it instead (int) dr.GetValue(0) (you may want to check for DBNull also)
  • If you only have one row and column in the result, you can use ExecuteScalar
  • You can merge these two queries into one and save yourself the round-trip of two batches
  • You are embedding the connection string in the code, it should be saved in a settings file
    private void button2_Click(object sender, EventArgs e) // PLACING ORDERS
    {
        int rowcount;
        const string query = @"
UPDATE Production.ProductInventory
SET Quantity -= @qty
WHERE ProductId = @productId
  AND Quantity >= @qty;
";
        using (SqlConnection con = new SqlConnection(Properties.Settings.ConnectionString))
        using (SqlCommand cmd1 = new SqlCommand(query,con))
        {
            cmd1.Parameters.Add("@productId", SqlDbType.Int).Value = int.Parse(textBox1.Text);
            cmd1.Parameters.Add("@qty", SqlDbType.Int).Value = qty;

            con.Open();
            rowcount = cmd.ExecuteNonQuery();
        }
        if(rowcount == 0)
        {
            MessageBox.Show("Unfortunately we are not able to provide you with the amount you want", "Oops!", MessageBoxButtons.OK, MessageBoxIcon.Warning);
        }
    }

Upvotes: 1

Related Questions