0

This code is working but I think something is missing because it only works if I add one row at a time. How can I store many rows at a time?

foreach (ListViewItem item in listView1.Items)
{
    for (int i = 0; i < listView1.Items.Count; i++)
    {
        SqlConnection con = new SqlConnection("Data Source=ZON-PC;Initial Catalog=RestaurantPOSSOC;Integrated Security=True");

        con.Open();

        SqlCommand _sqlcommand = new SqlCommand("insert into OrderInfo (ProductName,Quantity,Price)values('" + listView1.Items[i].SubItems[0].Text + "','" + listView1.Items[i].SubItems[1].Text + "','" + listView1.Items[i].SubItems[2].Text + "')", con);

        SqlDataReader _sqldatareader = _sqlcommand.ExecuteReader();
        _sqldatareader.Read();

       con.Close();
    }
}
Improve this question
5
  • You do not need the inner for() loop. The foreach will iterate over the entire collection and you can execute an INSERT for item.SubItems[0].Text and item.SubItems[1].Text. Commented Nov 16, 2015 at 17:14
  • This is also very insecure and open to sql injection attacks. You should parameterize your insert. Commented Nov 16, 2015 at 17:21
  • And you don't have to open your connection to the database and close it in each loop iteration. Commented Nov 16, 2015 at 17:22
  • use bulk insert or if that doesn't work then insert multiple rows using XML or create a user type and do it that way.. very simple and lots of existing working example out there on the web.. Commented Nov 16, 2015 at 17:36
  • what version of SQL server are you using? Commented Nov 16, 2015 at 17:51

5 Answers 5

Reset to default
1

There are a couple of things I would change for your routine:

  1. As gmiley commented, you don't need the foreach loop and the for loop, you are doing n*n inserts this way
  2. Connections can be expensive, I would create the connection outside of the foreach loop.
  3. When I am doing updates, I use SqlCommand.ExecuteNonQuery, instead of SqlCommand.ExecuteReader(), since I don't actually expect any rows to come back.

So in pseudocode:

using (connection = new(...))
    con.open
    foreach(item)
        command = new command()
        command.ExecuteNonQuery()

    con.close
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

This is a fairly good sum-up. For completeness, I would include the comments from @Agapwlesu regarding parameterizing the command. This has a number of benefits: 1. Security: by passing values as parameter objects you inherently remove SQL Injection as an attack vector. 2. Data Validation: As a parameter you can enforce data types on incoming values. 3. Readability: Having typed parameters using a standard naming convention will, as a very nice side effect of the other two benefits of parameterized queries, lead to very readable code with little need of comments.
@gmiley Agreed, but I was trying to limit my answer to the question posed. While those are important, they take the question to a whole different level that I think OP won't be ready for until understanding how the simpler code works. OP will need to get to that level, but one step at a time.
Yep, I just usually find that doing things in line with the .Net framework from the start usually makes your life easier when diagnosing problems like this. So if, by suggestion, I can get someone to refactor to a more proper style of writing, it makes everyone's job a little easier. No need to change your answer, mostly just wanted to have a comment on it to associate it with the other answers/comments, as they aren't answers themselves, but they are correct in content.
0

Look Here

basically: INSERT INTO Table ( Column1, Column2 ) VALUES ( Value1, Value2 ), ( Value1, Value2 )

Improve this answer

Comments

0

Not an answer, but you really should change your starting code to at least this:

SqlConnection con = new SqlConnection("Data Source=ZON-PC;Initial Catalog=RestaurantPOSSOC;Integrated Security=True");
con.Open();

foreach (ListViewItem item in listView1.Items)
{
    SqlCommand _sqlcommand = new SqlCommand("insert into OrderInfo (ProductName,Quantity,Price)values('" + item.SubItems[0].Text + "','" + item.SubItems[1].Text + "','" + item.SubItems[2].Text + "')", con);

    SqlDataReader _sqldatareader = _sqlcommand.ExecuteReader();
    _sqldatareader.Read();
}

con.Close();

And you really also need to parameterize your query and add some exception handling.

Improve this answer

Comments

0

First: regardless of what method is used, you really need to clean up the external resources. Both the SqlConnection and SqlCommand objects have a .Dispose() method and should be used in a using() construct / macro. The using handles both the .Dispose() as well as a basic try / finally to ensure that the .Dispose() method is called, even if an error occurs.

Second: if you are using SQL Server 2008 or newer, you should check out Table Valued Parameters (TVPs) as they will eliminate the SQL Injection issues and increase performance.

I have examples of doing this in the following answers:

Improve this answer

Comments

0

Add this under button click or any, before that you should open a SqlConnection 

conn = new SqlConnection(ConfigurationManager.ConnectionStrings["myConnectionString"].ConnectionString); 

foreach (ListViewItem item in lvPOItem.Items)
{
    SqlCommand cmd = new SqlCommand("INSERT INTO PurchasePOItems (PO_ItemName, PO_Specs, PO_PONumber)values(@PO_ItemName, @PO_Specs,@PO_PONumber)", conn);
    conn.Open();
    cmd.Parameters.AddWithValue("@PO_ItemName", item.SubItems[0].Text);
    cmd.Parameters.AddWithValue("@PO_Specs", item.SubItems[1].Text);
    cmd.Parameters.AddWithValue("@PO_PONumber", cbPurchaseOrderNo.Text);
    cmd.ExecuteNonQuery();
    conn.Close();
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.