C# - DataReader skipping first result
Looking to see if I can get some help on this. I have a BackgroundWorker that will perform a SQL query and then update a DataGridView during the progress changed event. However, it is skipping the first row and adding a duplicate last row to the DGV. The column headers are off as I added in the UID for troubleshooting. So please disregard how the titles do not match the data.
I checked the usual culprit of having a Read() in the IF, but that is not the case.
Interestingly, if I put a break on the reader = Sqlcmd.ExecuteReader(); and step through it, it randomly works.
Any help would be greatly appreciated! I'm at a complete loss.
Additionally, if I could move the DGV add from the progress changed to the complete event it would be a huge benefit as well.
/// DO WORK
private void issueBWworker_DoWork_1(object sender, DoWorkEventArgs e)
{
// DGV 1
RetriveTableData Obj = (RetriveTableData)e.Argument;
string SqlcmdString = "SELECT * FROM xBETA_OAP_ISSUE";
SqlDataReader reader;
int i = 1;
try
{
using (SqlConnection conn = new SqlConnection(ConnString))
{
Sqlcmd = new SqlCommand(SqlcmdString, conn);
conn.Open();
reader = Sqlcmd.ExecuteReader();
if (reader.HasRows)
{
while (reader.Read())
{
Obj.uid = reader["id"].ToString();
Obj.iss_type = reader["issue_type"].ToString();
Obj.inc_num = reader["ticket_num"].ToString();
Obj.create_date = reader["create_date"].ToString();
Obj.created_by = reader["created_by"].ToString();
Obj.active = reader["active"].ToString();
Obj.change_date = reader["change_date"].ToString();
Obj.changed_by = reader["changed_by"].ToString();
Thread.Sleep(100);
//MessageBox.Show(Obj.uid);
// To Report progress.x
issueBWworker.ReportProgress(i, Obj);
if (issueBWworker.CancellationPending)
{
// Set the e.Cancel flag so that the WorkerCompleted event
// knows that the process was cancelled.
e.Cancel = true;
issueBWworker.ReportProgress(0);
return;
}
i++;
}
conn.Close();
}
}
}
catch (Exception ex)
{
MessageBox.Show(ex.Message);
}
}
/// Progress Changes
private void issueBWworker_ProgressChanged_1(object sender, ProgressChangedEventArgs e)
{
if (!issueBWworker.CancellationPending)
{
// DGV 1
RetriveTableData Obj = (RetriveTableData)e.UserState;
issue_dgv.Rows.Add(Obj.uid.ToString(), Obj.iss_type.ToString(), Obj.inc_num.ToString(), Obj.create_date.ToString(), Obj.created_by.ToString(), Obj.active.ToString(), Obj.change_date.ToString(), Obj.changed_by.ToString());
pbar.Value = e.ProgressPercentage;
//toolStripStatusLabel1.Text = "Processing row.. " + e.ProgressPercentage.ToString() + " of " + TotalRecords;
}
}
/// Complete
private void issueBWworker_RunWorkerCompleted_1(object sender, RunWorkerCompletedEventArgs e)
{
if (e.Cancelled)
{
//toolStripStatusLabel1.Text = "Cancelled by User Intentionally...";
pbar.Value = 0;
pbar.Visible = false;
}
// Check to see if an error occurred in the background process.
else if (e.Error != null)
{
//toolStripStatusLabel1.Text = e.Error.Message;
pbar.Visible = false;
}
else
{
// BackGround Task Completed with out Error
pbar.Visible = false;
//toolStripStatusLabel1.Text = " All Records Loaded...";
}
}
According to the documentation, in the remarks section it mentions that the call to ReportProgress is asynchronous and returns immediately. However, in order for it to work as expected with your code, you'd need it to run synchronously.
This is because you're using the same object, Obj, to pass the data on each iteration of your reader to ReportProgress - and possibly using Thread.Sleep(100) as a means of synchronisation - this is not really a valid or effective design, and probably resulting in the behaviour you're seeing.
For example, let's assume the data has four rows, this would be the outcome:
+--------+-----------------------------------+--------------------------------------------+
| | DoWork | ReportProgress |
+--------+-----------------------------------+--------------------------------------------+
| 1 | Populate values of Obj with row 1 | |
+--------+-----------------------------------+--------------------------------------------+
| 2 | Wait 100 ms | |
+--------+-----------------------------------+--------------------------------------------+
| 3 | Dispatch ReportProgress | |
+--------+-----------------------------------+--------------------------------------------+
| 4 | Populate values of Obj with row 2 | |
+--------+-----------------------------------+--------------------------------------------+
| 5 | Wait 100 ms | Add a row with values of Obj, now at Row 2 |
+--------+-----------------------------------+--------------------------------------------+
| 6 | Dispatch ReportProgress | |
+--------+-----------------------------------+--------------------------------------------+
| 7 | Populate values of Obj with row 3 | |
+--------+-----------------------------------+--------------------------------------------+
| 8 | Wait 100 ms | Add a row with values of Obj, now at Row 3 |
+--------+-----------------------------------+--------------------------------------------+
| 9 | Dispatch ReportProgress | |
+--------+-----------------------------------+--------------------------------------------+
| 10 | Populate values of Obj with row 4 | |
+--------+-----------------------------------+--------------------------------------------+
| 11 | Wait 100 ms | Add a row with values of Obj, now at Row 4 |
+--------+-----------------------------------+--------------------------------------------+
| 12 | Dispatch ReportProgress | |
+--------+-----------------------------------+--------------------------------------------+
| 13 | End of iteration, no more rows | Add a row with values of Obj, now at Row 4 |
+--------+-----------------------------------+--------------------------------------------+
How to fix it?
The easiest way would be to use a new instance of Obj on each iteration. That way, that way each call to ReportProgress has its own instance which will not be changed.
In other words, something like the below assuming RetrieveTableData is a simple object requiring no parameters to its constructor. I'm deliberately keeping to your original style rather than rewriting everything.. there are lots of issues we could talk about here, code style, names of variables, but it is not directly relevant to the problem...
while (reader.Read())
{
var Obj = new RetriveTableData(); // create a new instance
Obj.uid = reader["id"].ToString();
Obj.iss_type = reader["issue_type"].ToString();
...
We would then no longer need this line at the top of the method; the point is to change the scope of Obj so we do not reuse it on each iteration.
RetriveTableData Obj = (RetriveTableData)e.Argument;
Thread.Sleep(100) would no longer be needed either.
To tackle your additional aim of creating the data rows in the complete event rather than the progress event, you could collect these individual RetrieveTableData objects into a List<RetrieveTableData>, and pass that to your RunWorkerCompleted event handler via e.Result on the DoWorkEventArgs.