Inserting records into a database

  • I wrote the following code to insert records into a database. The user selects the rows from the RadGrid and the insert command is executed when that user clicks the button. I've been spending a lot of time trying to make this work (nothing currently happens when I click the button). However, I just realized my approach is probably very inefficient due to the fact that it will result in the application repeatedly opening and closing database connections. Even though I'm dealing with only a few hundred records I'd like to follow best practices as much as possible.



    How would you rewrite this code to be more efficient? My gut tells me it would be better to gather all of the new records and then deal with them as a collection.



        protected void RadButton2_Click(object sender, EventArgs e)
    {
    foreach (GridDataItem item in RadGrid1.SelectedItems)
    {
    //GridDataItem item = (GridDataItem)RadGrid1.SelectedItems;
    int UserID = Convert.ToInt16(item["UserID"].Text);
    string Type = "D";
    DateTime Date = DateTime.Now;
    string Description = "Monthly Storage Fee - Tag: " + item["PackageTag"].Text + Label3.Text;
    Int32 AmountDue = Convert.ToInt32(item["AmtDue"].Text);


    string connectionString = ConfigurationManager.ConnectionStrings["Foo"].ConnectionString;
    SqlConnection connection = new SqlConnection(connectionString);

    try
    {

    SqlCommand cmd = new SqlCommand("INSERT INTO Billing (UserID, Type, Date, Description, Amount) VALUES (@UserID, @Type, @Date, @Description, @AmountDue)", connection);
    cmd.Parameters.AddWithValue("@UserID", UserID);
    cmd.Parameters.AddWithValue("@Type", Type);
    cmd.Parameters.AddWithValue("@Date", Date);
    cmd.Parameters.AddWithValue("@Description", Description);
    cmd.Parameters.AddWithValue("@AmountDue", AmountDue);

    connection.Open();
    cmd.ExecuteNonQuery();
    }

    catch
    {
    Label4.Text = "uh oh";
    }

    finally
    {
    connection.Close();
    }

    ADO.NET will actually not dispose or open/close the connection behind the scenes but decides whether to reuse an existing connection with enabled Connection Pooling(default). So yes, it's recommendet to open/close connections or use the using-keyword.

    Are you by chance connecting to a SQL Server 2008 database? If so, you could work with the database developers to create a stored procedure that takes a table variable as an input parameter and inserts all the rows into the database at the same time.

    Yes I am actually. Interesting suggestion. I'm going to investigate that. For this project _I_ am the database developer too.

    Added some articles to get you started with TVP's as an answer.

  • Aristos

    Aristos Correct answer

    10 years ago

    This is not best practice but simple logic.



    Why you have add inside the loop this (non change) parameters ?



        string Type = "D";
    DateTime Date = DateTime.Now;
    string connectionString =
    ConfigurationManager.ConnectionStrings["Foo"].ConnectionString;


    Get them out of the loop.



    Second place the connection inside using, and totally outside the loop, why you need to open it and close it all the time.



    protected void RadButton2_Click(object sender, EventArgs e)
    {
    //GridDataItem item = (GridDataItem)RadGrid1.SelectedItems;
    string Type = "D";
    DateTime Date = DateTime.Now;

    string connectionString = ConfigurationManager.ConnectionStrings["Foo"].ConnectionString;
    SqlConnection connection = new SqlConnection(connectionString);

    using(SqlConnection connection = new SqlConnection(connectionString))
    {
    connection.Open();
    foreach (GridDataItem item in RadGrid1.SelectedItems)
    {
    try
    {
    string Description = "Monthly Storage Fee - Tag: " + item["PackageTag"].Text + Label3.Text;
    Int32 AmountDue = Convert.ToInt32(item["AmtDue"].Text);
    int UserID = Convert.ToInt16(item["UserID"].Text);

    using (SqlCommand cmd = new SqlCommand("INSERT INTO Billing (UserID, Type, Date, Description, Amount) VALUES (@UserID, @Type, @Date, @Description, @AmountDue)", connection))
    {
    cmd.Parameters.AddWithValue("@UserID", UserID);
    cmd.Parameters.AddWithValue("@Type", Type);
    cmd.Parameters.AddWithValue("@Date", Date);
    cmd.Parameters.AddWithValue("@Description", Description);
    cmd.Parameters.AddWithValue("@AmountDue", AmountDue);

    cmd.ExecuteNonQuery();
    }
    }
    catch
    {
    Label4.Text = "uh oh";
    }
    }
    }
    }


    Need some extra test for check if the connection is open or fail


    Aristos, can you spot anything I may have done wrong? When I run the app nothing actually inserts into the database. I'm not noticing any exceptions either. edit: I'm going with your recommended code by the way.

    @hughesdan I belive that fail in the date, you need to format it in the SQL way, or declare that is a date. As you add it now, its convert it to string, and probably fails to insert.

    That's probably what it is. And come to thing of it I could also use SQL GetDate().

    `SqlCommand` also implements `IDisposable` and should be placed within a `using` block.

    @JesseC.Slicer Yes you have right, if you like please update the code.

License under CC-BY-SA with attribution


Content dated before 7/24/2021 11:53 AM

Tags used