C# win forms app hangs

Question

The following code is for reading from an Access DB and transferring to a SQL DB.

Can anyone tell me why this code just hangs after it is run ?

private void readWriteRefuseDay()
{
    OleDbDataReader dr;
    oCon = new OleDbConnection(oConStr);
    sqlCon = new SqlConnection(sqlConStr);
    oQuery = "SELECT UPRN, RefuseDay, RefuseWeek FROM RefuseDay";
    sqlQuery = "INSERT INTO Ref_RefuseDay (UPRN, RefuseDay, RefuseWeek) VALUES (@UPRN, @RefuseDay, @RefuseWeek)";
    oCmd = new OleDbCommand(oQuery, oCon);

    string sUPRN;
    string sRefuseDay;
    Int32 iRefuseWeek;

    try
    {
        oCon.Open();
        sqlCon.Open();
        count = 0;
        lblProcessing.Text = count.ToString();

        dr = oCmd.ExecuteReader();

        while (dr.Read())
        {
            lblProcessing.Text = "Processing: RefuseDay " + count.ToString();
            sUPRN = dr.GetString(0);
            sRefuseDay = dr.GetString(1);
            iRefuseWeek = dr.GetInt32(2);

            sqlCmd = new SqlCommand(sqlQuery, sqlCon);
            sqlCmd.Parameters.AddWithValue("@UPRN", sUPRN);
            sqlCmd.Parameters.AddWithValue("@RefuseDay", sRefuseDay);
            sqlCmd.Parameters.AddWithValue("@RefuseWeek", iRefuseWeek);
            sqlCmd.ExecuteNonQuery();

            count++;
        }

        dr.Close();

        oCon.Close();
        oCmd.Dispose();
        oCon.Dispose();

        sqlCon.Close();
        sqlCmd.Dispose();
        sqlCon.Dispose();
    }
    catch (Exception ex) { MessageBox.Show(ex.Message); }

I have a label that should change telling me the row that is being processed but the Form just sits and becomes unresponsive once I have clicked the button.

Everything works, its just the form that freezes.

I'm open to any and all suggestions to improve the code also. I've noticed people using:

using (SqlConnection sqlCon = new SqlConnection(connectionString)) 

for example. Never really understood this. Is it better ? I have seen it used for SqlCommand objects too.

Any way this code can be improved, lets hear it.

Thanks in advance.

Answer 1

1. You should always use using statements on anything that implements IDisposable, it ensures the resource is released even in the event of an Exception. In this case it ensures your DB Connections are closed.
2. Put your Ado.Net types in the scope of the method, not the class. Do not worry about trying to reuse connection objects, it is almost always a bad idea and most RDBMs like Sql Server handle pooling of connections so you do not have to.
3. Your screen can appear to hang if this takes a while. The best solution is to make your event handler (button click for example) mark with async. What the signature looks like can depend on the UI (winforms, wpf, something else) but the idea is to offload the work from the main message window. As it is not immediately clear from what and where this is being called I did not update the code to reflect this.

const string oQuery = "SELECT UPRN, RefuseDay, RefuseWeek FROM RefuseDay";
const string sqlQuery = "INSERT INTO Ref_RefuseDay (UPRN, RefuseDay, RefuseWeek) VALUES (@UPRN, @RefuseDay, @RefuseWeek)";
try
{
    using(var oCon = new OleDbConnection(oConStr))
    using(var sqlCon = new SqlConnection(sqlConStr))
    using(var oCmd = new OleDbCommand(oQuery, oCon))
    {
        oCon.Open();
        sqlCon.Open();
        count = 0;
        lblProcessing.Text = count.ToString();

        using(var dr = oCmd.ExecuteReader())
        {
            while (dr.Read())
            {
                lblProcessing.Text = "Processing: RefuseDay " + count.ToString();
                var sUPRN = dr.GetString(0);
                var sRefuseDay = dr.GetString(1);
                var iRefuseWeek = dr.GetInt32(2);

                using(var sqlCmd = new SqlCommand(sqlQuery, sqlCon))
                {
                   sqlCmd.Parameters.AddWithValue("@UPRN", sUPRN);
                   sqlCmd.Parameters.AddWithValue("@RefuseDay", sRefuseDay);
                   sqlCmd.Parameters.AddWithValue("@RefuseWeek", iRefuseWeek);
                   sqlCmd.ExecuteNonQuery();
                }
                count++;
            }
        }
    }
}
catch (Exception ex) { 
  MessageBox.Show(ex.Message); 
}

---

Edit with Task and async/await

In order to aleviate the freezing window you can take advantage of several async methods provided on the ado.net types. When used with async/await when an await is encountered execution is suspended and control returned to the main message window. Here is that same code but updated so that it takes advantage of the async methods. It also illustrates how you call it from a button click method.

private async void button_Click(object sender, EventArgs e)
{
    await readWriteRefuseDayAsync();
}

private async Task readWriteRefuseDayAsync() {
  const string oQuery = "SELECT UPRN, RefuseDay, RefuseWeek FROM RefuseDay";
  const string sqlQuery = "INSERT INTO Ref_RefuseDay (UPRN, RefuseDay, RefuseWeek) VALUES (@UPRN, @RefuseDay, @RefuseWeek)";
  try {
    using(var oCon = new OleDbConnection(oConStr))
    using(var sqlCon = new SqlConnection(sqlConStr))
    using(var oCmd = new OleDbCommand(oQuery, oCon))
    {
        await oCon.OpenAsync();
        await sqlCon.OpenAsync();
        count = 0;
        lblProcessing.Text = count.ToString();

        using(var dr = await oCmd.ExecuteReaderAsync())
        {
            while (await dr.ReadAsync())
            {
                lblProcessing.Text = "Processing: RefuseDay " + count.ToString();

                var sUPRN = dr.GetString(0);
                var sRefuseDay = dr.GetString(1);
                var iRefuseWeek = dr.GetInt32(2);

                using(var sqlCmd = new SqlCommand(sqlQuery, sqlCon))
                {
                   sqlCmd.Parameters.AddWithValue("@UPRN", sUPRN);
                   sqlCmd.Parameters.AddWithValue("@RefuseDay", sRefuseDay);
                   sqlCmd.Parameters.AddWithValue("@RefuseWeek", iRefuseWeek);
                   await sqlCmd.ExecuteNonQueryAsync();
                }
                count++;
            }
        }
    }
  }
  catch (Exception ex) {
    MessageBox.Show(ex.Message); 
  }
}