ExecuteNonQuery() breaks the loop and doesn't insert data in SQL database

Question

Here's the code which tries to enter the data stored in the arrays. The arrays contain data as well as there are empty cells which need not be added into the database. The issue is the code is not throwing any exception or error but it isn't inserting any data in the database too! Please help... Thanks in advance

public void saveDb(string[,] timeTableId,string[,] start_time,string[,] end_time,string[,] subject_id,string[,] day,string[,] faculty_id)
{
    SqlConnection con;
    SqlCommand cmd;
    con = new SqlConnection("Data Source=.;Initial Catalog=AIS;Integrated Security=True");
    con.Open();
    for (int i = 0; i < 8; i++)
    {
        for (int j = 1; j <= 7; j++)
        {
            if (subject_id[i, j].Length != 0 && subject_id[i, j] != null)
            {
                cmd = new SqlCommand("INSERT INTO TIMETABLE VALUES('" + subject_id[i, j] + "','" + day[i, j] + "','" + start_time[i, j] + "','" + end_time[i, j] + "','" + subject_id[i, j] + "','" + faculty_id[i, j] + "')", con);
                cmd.ExecuteNonQuery();
            }
            else
            { 
            }
        }
    }
    con.Close();
}

Answer 1

Try to capture the SQL statement using SQL Profiler, and then, run the query on SQL Management Studio to discover if there's any error on it.

Answer 2

Try:

Use parameterized Query and use Try catch block to get the exception if Any

public void saveDb(string[,] timeTableId,string[,] start_time,string[,] end_time,string[,] subject_id,string[,] day,string[,] faculty_id)
{
 SqlConnection con;
  SqlCommand cmd;
    con = new SqlConnection("Data Source=.;Initial Catalog=AIS;Integrated Security=True");
try
{
   if(con.State == ConnectionState.Closed)
          con.Open();

    for (int i = 0; i < 8; i++)
    {
        for (int j = 1; j <= 7; j++)
        {
            if (subject_id[i, j].Length != 0 && subject_id[i, j] != null)
            {
                cmd = new SqlCommand("INSERT INTO [TIMETABLE](col1,col2,col3,col4,col5,col6) VALUES(@col1,@col2,@col3,@col4,@col5,@col6)", con);

cmd.Parameter.AddWithValue("@col1",subject_id[i, j]);
   // convert type here depend upon the col type
    // cmd.Parameter.AddWithValue("@col1",Convert.ToString(subject_id[i, j]));
   //Or  cmd.Parameter.AddWithValue("@col1",Convert.ToDouble(subject_id[i, j]));
cmd.Parameter.AddWithValue("@col2",day[i, j]);
cmd.Parameter.AddWithValue("@col3",start_time[i, j]);
cmd.Parameter.AddWithValue("@col4",end_time[i, j]);
cmd.Parameter.AddWithValue("@col5",subject_id[i, j]);
cmd.Parameter.AddWithValue("@col6",faculty_id[i, j]);

                cmd.ExecuteNonQuery();
            }
        }
    }
}
Catch(Exception e1)
{
  throw new System.ArgumentException(e1.Messege, "Error");
}
Finally
{
   if(con.State == ConnectionState.Open)
        con.Close();
}
}

Answer 3

OK, I'm elaborating....

1. use parametrized queries - first to avoid SQL injection, the #1 vulnerability out there on the internet, second to avoid issues with how many single or double quotes do I need for this string or date? and stuff like that - gone if you use properly typed parameters, and third to improve performance - define your parameter once, re-use them multiple times (and SQL Server will also create one SQL statement with an execution plan and reuse that!)

2. use **using(....) { .... } blocks for all disposable classes - especially SqlConnection, SqlCommand, SqlDataReader - to ensure proper and immediate disposal of unneeded objects.

3. always explicitly define the list of columns of a table you're inserting into - don't just rely on the current table structure and order of columns - explicitly say what you're doing!

All in all, your method should really look something like this:

public void saveDb(string[,] timeTableId,string[,] start_time,string[,] end_time,string[,] subject_id,string[,] day,string[,] faculty_id)
{
    // define connection string - typically should come from a .config file
    string connectionString = "Data Source=.;Initial Catalog=AIS;Integrated Security=True";

    // define the SQL query - with *parameters* - and also: explicitly NAME the columns in your target table!
    // also: did you really want to insert the subject_id twice?
    string insertQry = "INSERT INTO dbo.TIMETABLE (col1, col2, col3, ....) " + 
                       " VALUES(@subject_id, @day, @start_time, @end_time, @subject_id, @faculty_id)";

    // set up your connection and command    
    // you didn't tell us what datatypes those are - maybe you need to adapt those to your situation!
    using (SqlConnection con = new SqlConnection(connectionString))
    using (SqlCommand cmd = new SqlCommand(insertQry, con))
    {
        // define your parameters once, before the loop
        cmd.Parameters.Add("@subject_id", SqlDbType.Int);
        cmd.Parameters.Add("@day", SqlDbType.DateTime);
        cmd.Parameters.Add("@start_time", SqlDbType.Time);
        cmd.Parameters.Add("@end_time", SqlDbType.Time);
        cmd.Parameters.Add("@faculty_id", SqlDbType.Int);

        con.Open();

        // now start the for loops, and set the parameter values        
        for (int i = 0; i < 8; i++)
        {
            for (int j = 1; j <= 7; j++)
            {
                // not sure what these checks should be - left them "as is"
                if (subject_id[i, j].Length != 0 && subject_id[i, j] != null)
                {
                     // set the parameter values
                     cmd.Parameters["@subject_id"].Value = subject_id[i, j];
                     cmd.Parameters["@day"].Value = day[i, j];
                     cmd.Parameters["@start_time"].Value = start_time[i, j];
                     cmd.Parameters["@end_time"].Value = end_time[i, j];
                     cmd.Parameters["@faculty_id"].Value = faculty_id[i, j];

                     // execute query to insert data                     
                     cmd.ExecuteNonQuery();
                }    
            }
        }

        con.Close();
    }
}