How to store correctly list of objects it database?

Question

I work on project that was created about 8 years ago it use ADO.NET technology.

The data that I want to save in the table is List of objects:

Here is custom class:

public class ReportTrafficDepartment
{
    public int id { get; set; }
    public System.Nullable<int> siteNum { get; set; }
    public System.Nullable<System.DateTime> dateReport { get; set; }
    public string siteName { get; set; }
    public System.Nullable<int> prog1 { get; set; }
    public System.Nullable<int> progLayout1 { get; set; }
    public System.Nullable<int> prog2 { get; set; }
    public System.Nullable<int> progLayout2 { get; set; }
    public System.Nullable<bool> isLOZ { get; set; }
    public System.Nullable<System.DateTime> start { get; set; }
    public System.Nullable<System.DateTime> end { get; set; }
    public System.Nullable<System.TimeSpan> time { get; set; }
    public System.Nullable<float> Prog1ToProg2Check { get; set; }
    public string comment { get; set; }
}

And here is the function that called when I want to save data:

    public void saveReport(IEnumerable<ReportTrafficDepartment> report)
    {
        try
        {

            SqlConnection conn = new SqlConnection("connetcion string");

            foreach (var record in report)
            {
                SqlCommand cmd = new SqlCommand("SaveEcxelReport", conn);
                cmd.CommandType = CommandType.StoredProcedure;

                cmd.Parameters.Add("@siteNum", SqlDbType.Int).Value = record.siteNum;
                cmd.Parameters.Add("@dateReport", SqlDbType.DateTime).Value = record.dateReport;
                cmd.Parameters.Add("@siteName", SqlDbType.NVarChar).Value = record.siteName;
                cmd.Parameters.Add("@prog1", SqlDbType.Int).Value = record.prog1;
                cmd.Parameters.Add("@progLayout1", SqlDbType.Int).Value = record.progLayout1;
                cmd.Parameters.Add("@prog2", SqlDbType.NVarChar).Value = record.prog2;
                cmd.Parameters.Add("@progLayout2", SqlDbType.Int).Value = record.progLayout2;
                cmd.Parameters.Add("@isLOZ", SqlDbType.Bit).Value = record.isLOZ;
                cmd.Parameters.Add("@start", SqlDbType.DateTime).Value = record.start;
                cmd.Parameters.Add("@end", SqlDbType.DateTime).Value = record.end;
                cmd.Parameters.Add("@Prog1ToProg2Check", SqlDbType.Real).Value = record.Prog1ToProg2Check;
                cmd.Parameters.Add("@comment", SqlDbType.NVarChar).Value = record.comment;

                conn.Open();
                cmd.ExecuteNonQuery();
                conn.Close();
            }
        }
        catch
        {
            throw;
        }
    }

And here is stored procedure that I use to insert record to the table:

      ALTER PROCEDURE [dbo].[SaveEcxelReport]
         @siteNum INT = NULL,
         @dateReport DATETIME = NULL,
         @siteName NVARCHAR(255) = NULL,
         @prog1 INT = NULL,
         @progLayout1 INT = NULL,
         @prog2 INT = NULL,
         @progLayout2 INT = NULL,
         @isLOZ BIT = NULL,
         @start DATETIME = NULL,
         @end DATETIME = NULL,
         @time DATETIME = NULL,
         @Prog1ToProg2Check REAL = NULL,
         @comment NVARCHAR(255) = NULL

        AS   
        BEGIN
          SET NOCOUNT ON;
          insert into dbo.ReportTrafficDepartment(siteNum, dateReport, siteName, prog1, progLayout1, prog2, progLayout2, isLOZ, [start], [end], [time], Prog1ToProg2Check, comment) 
                 values (@siteNum, @dateReport, @siteName, @prog1,@progLayout1, @prog2, @progLayout2, @isLOZ, @start, @end, @time, @Prog1ToProg2Check, @comment) 
        END

As you can see in SaveReport function in foreach method I open connection I pass item to the stored procedure then I close connection.

But, I think my attitude is wrong (i.e. it's not good idea to open and close connection inside foreach loop).

Any idea should I change the attitude to store collection to the database if yes how should I do it?

Answer 1

You should add a using statement around the creation of the SqlConnection and open it before entering the loop. The using statement will close automatically the connection when you exit from that using block. Another little step is to create the parameters outside the loop just one time and set the values inside the loop. No need to create all that parameter set at each loop, you can reuse them. But the most important fix is to add a transaction around your code to prevent partial inserts of your enumerable in case of exceptions.

public void saveReport(IEnumerable<ReportTrafficDepartment> report)
{
    try
    {

        using(SqlConnection conn = new SqlConnection("connetcion string"))
        {
            conn.Open();
            using(SqlTransaction tr = conn.BeginTransaction())
            using(SqlCommand cmd = new SqlCommand("SaveEcxelReport", conn, tr))
            {
                 cmd.CommandType = CommandType.StoredProcedure; 
                 cmd.Parameters.Add("@siteNum", SqlDbType.Int);
                 cmd.Parameters.Add("@dateReport", SqlDbType.DateTime);
                 .... other parameters ....
                foreach (var record in report)
                {
                    cmd.Parameters["@siteNum"].Value = record.siteNum;
                    cmd.Parameters["@dateReport"].Value = record.dateReport;

                    ... set the values for other parameters

                    cmd.ExecuteNonQuery();
               }
               tr.Commit();
            }
        }
    }
    catch
    {
        // If something goes wrong the rollback is executed automatically 
        // when you exit from the using block above
        //if(tr != null) 
        //{
        //   tr.RollBack();
        //}

        // ------
        // Here you should log the error before throwing. 
        // It is a good habit and often a lifesaver to log the errors 
        // ------

        // But if you don't log or you don't do anything else 
        // but just throw then the whole try catch is useless 
        // and you can completely remove it 
        throw;
    }
}

After these simple fixes you should look at using an ORM that can just take the enumerable and store it to the database without further coding on your part