foreach is returning only 1st id and duplicating for remaining ids

Question

I am running nested foreach, where first I want to get all id and then for each id I want to populate datagridview, but the it is returning only first id six times. (my tables has only six data rows). Ran diagnostics with breakpoints, DataTable dt0 has all ids, but DataTable dt2 has only first id. same goes for other var.

int id = 0;
int price = 0;
int qty = 0;


private void button1_Click(object sender, EventArgs e)
{
    con.Open();
    string sql = "SELECT id FROM tbl_price ";
    SqlCommand cmd0 = new SqlCommand(sql, con);
    SqlDataAdapter dAdapter0 = new SqlDataAdapter(cmd0);
    DataTable dt0 = new DataTable();

    dAdapter0.Fill(dt0);
    if (dt0.Rows.Count > 0)
    {
        foreach (DataRow dr in dt0.Rows)
        {
            SqlCommand cmd2 = new SqlCommand("SELECT P.id, P.price, Q.qty FROM tbl_price P INNER JOIN tbl_qty Q ON P.id=Q.id WHERE P.id='" + dt0.Rows[0]["id"] + "'  ", con);
            SqlDataAdapter dAdapter2 = new SqlDataAdapter(cmd2);
            DataTable dt2 = new DataTable();
            dAdapter2.Fill(dt2);

            id = Convert.ToInt32(dt2.Rows[0]["id"]);
            price = Convert.ToInt32(dt2.Rows[0]["price"]);
            qty = Convert.ToInt32(dt2.Rows[0]["qty"]);

            int n = dataGridView1.Rows.Add();
            dataGridView1.Rows[n].Cells["dgid"].Value = id;
            dataGridView1.Rows[n].Cells["dgprice"].Value = price;
            dataGridView1.Rows[n].Cells["dgqty"].Value = qty;

        }
    }
    con.Close();
}

//Below is my result in Datagridview

   id   qty price
    1   100 10  
    1   100 10  
    1   100 10  
    1   100 10  
    1   100 10  
    1   100 10

And why don't you just do a single join? No nesting of anything should be required here. — Aaron Bertrand, Aug 18 '20 at 16:28

score 1 · Answer 1 · answered Aug 18 '20 at 16:31

Without knowing the structure of the data in your SQL, it's a little difficult to tell, but I'm suspicious of the line SqlCommand cmd2 = new SqlCommand("SELECT P.id, P.price, Q.qty FROM tbl_price P INNER JOIN tbl_qty Q ON P.id=Q.id WHERE P.id='" + dt0.Rows[0]["id"] + "' ", con);

This implies to me that, despite looping through the rows, your second query is always using the first element of dt0.Rows.

I would try replacing dt0.Rows[0]["id"] with dr["id"].

Replacing dt0.Rows[0]["id"] with dr["id"] worked. Thanx alot — Devilpor, Aug 18 '20 at 18:37

score 0 · Answer 2 · answered Aug 18 '20 at 16:31

0

dt0.Rows[0]["id"]

Gives you still the same first row. You´d need some iterator if you want to loop through all the rows.

So you´d be better off with using for rather than foreach, especially because you don´t use the DataRow dr.

answered Aug 18 '20 at 16:31

krysta24

23
1
7

Soenhay · Answer 3 · 2020-08-18T16:58:09.537

I am assuming that your query returns 1 row per id from tbl_qty. If not then this might need to change.

Some notes:

You do not need to run 2 queries for this function.
While string concatenation can open you up for SQL injection you can remove the concatenation in this function by using 1 query.
The test for rows > 0 is unnecessary when using foreach.
It is also good practice to wrap the connection work in a "try" and close the connection in a "finally" to make sure the connection is closed if something fails.

Simplifying the code as follows might help:

        con.Open();
        try
        {
            string sql = "SELECT P.id, P.price, Q.qty FROM tbl_price P INNER JOIN tbl_qty Q ON P.id=Q.id";
            SqlCommand cmd0 = new SqlCommand(sql, con);
            SqlDataAdapter dAdapter0 = new SqlDataAdapter(cmd0);
            DataTable dt0 = new DataTable();
            dAdapter0.Fill(dt0);

            foreach (DataRow dr in dt0.Rows)
            {
                id = Convert.ToInt32(dr["id"]);
                price = Convert.ToInt32(dr["price"]);
                qty = Convert.ToInt32(dr["qty"]);

                int n = dataGridView1.Rows.Add();
                dataGridView1.Rows[n].Cells["dgid"].Value = id;
                dataGridView1.Rows[n].Cells["dgprice"].Value = price;
                dataGridView1.Rows[n].Cells["dgqty"].Value = qty;
            }
        }
        finally
        {
            con.Close();
        }

See this answer for different ways to set the dataGridView row values and shorten the code further.

One reason for shortening the code is to remove some of the complexity to make it easier to debug.

foreach is returning only 1st id and duplicating for remaining ids

3 Answers3