0

The current program I am building is used to save invoices and I want to save data into a database. However instead of repeating this code shown below 20 times for each possible entry i would like to create a function with the text box name changing in the function.

All the text boxes are named with a number at the end from 1 to 20. I was wondering if there is a way to have a function that would change the number at the end and if its even worth doing compared to repeating this 20 times.

if (txtProductID1.Text.Length > 0)
        {
            OleDbConnection oledbconnection1 = new OleDbConnection();
            oledbconnection1.ConnectionString = Con;
            OleDbCommand cmd;

            String strInsert = "";
            //Generate SQL Statement
            strInsert = "Insert into [InvoiceOrder] Values (";
            strInsert += "'1', ";
            strInsert += "'" + txtInvoiceNo.Text + "', ";
            strInsert += "'" + txtProductDescription1.Text + "', ";
            strInsert += "'" + txtOrderNo1.Text + "', ";
            strInsert += "'" + cboUnit1.Text + "', ";
            strInsert += "'" + txtAmount1.Text + "', ";
            strInsert += "'" + txtPrice1.Text + "', ";
            strInsert += "'" + txtSum1.Text + "', ";
            strInsert += "'" + txtDiscount1.Text + "' ";
            strInsert += ")";
            try
            {
                oledbconnection1.Open();
                cmd = new OleDbCommand();
                cmd.CommandText = strInsert;
                cmd.Connection = oledbconnection1;
                cmd.ExecuteNonQuery();
                //MessageBox.Show("Record saved");
            }
            catch (Exception ex)
            {
                MessageBox.Show("Error : " + ex.ToString());
            }
            finally
            {
                oledbconnection1.Close();
            }
        }
Ryan Emerle
  • 15,461
  • 8
  • 52
  • 69
rupislt
  • 3
  • 1
  • 3
    Do it in a loop... You should also really look into SQL injection and why it's not a very good thing. It's good that you are spotting ways to not duplicate your code though. :) – Steve May 28 '14 at 21:27
  • 3
    OMG, have you ever heard about Sql Injection? And what happen if the product description contains a single quote? Learn how to use a parameterized query. You will be happy after – Steve May 28 '14 at 21:27
  • [Parameterized Queries and SQL Injection](http://stackoverflow.com/questions/5468425/how-do-parameterized-queries-help-against-sql-injection) – Ryan Emerle May 28 '14 at 21:37
  • Thank you for the advice I will look into it so that I make sure everything is OK in the future. But for this instance I think I will disable character entry for the entry of each value. Or update it in the future. – rupislt May 28 '14 at 22:04

1 Answers1

0

First, parameterize your query. Aside from security, the query you're building up is going to trip you up when you forget a single apostrophe somewhere.

As for iterating through the controls, perhaps Controls.Find() will work for you. The following code assumes all controls have a number from 1 to 20, and each number occurs once and only once on the form. (In your example, txtInvoiceNo does not have a number - I assume that's a typo.)

I made a few other changes too, like replacing your finally block with a using block, which will close and dispose your connection for you.

for (var i = 1; i <= 20; i++)
{    
    if (!String.IsNullOrEmpty(Controls.Find("txtProductID" + i, true).Single().Text))
    {
        using (var oledbconnection1 = new OleDbConnection())
        {
            oledbconnection1.ConnectionString = Con;
            oledbconnection1.Open();

            var insertStatement =
                "Insert into [InvoiceOrder] Values ('1', @InvoiceNo, @ProductDesc, @OrderNo, @Unit, @Amount, @Price, @Sum, @Discount)";

            try
            {
                using (var cmd = new OleDbCommand(insertStatement, oledbconnection1))
                {
                    cmd.Parameters.AddWithValue("@InvoiceNo", Controls.Find("txtInvoiceNo" + i, true).Single().Text);
                    ...
                    ...
                    cmd.Parameters.AddWithValue("@Discount", Controls.Find("txtDiscount" + i, true).Single().Text);
                    cmd.ExecuteNonQuery();
                    //MessageBox.Show("Record saved");
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show("Error : " + ex.ToString());
            }
        }
    }
}
Grant Winney
  • 65,241
  • 13
  • 115
  • 165
  • Thank you very much this helped a-lot. Also I am not aware of injection at all since I am a student and this was the way I learned from my lecturer. I only know 2 methods and I was told that the injection method was better compared to the wizards. Finally, the invoice number is a non changing textbox but I am sure i will figure out a way to do it on my own. – rupislt May 28 '14 at 21:56
  • Ok i almost got it working just 1 problem that comes up with the if statement"(!String.IsNullOrEmpty(Controls.Find("txtProductID" + i, false).Single().Text))""Sequence contains no elements". I tried to change Single to SingleOrDefault as suggested in one of the forums didn't help. I dont know if this is something I am doing or missing. – rupislt May 28 '14 at 22:28