Progress bar exceeding max value

Question

Progress bar exceeds max value when using the ++ operate to increment the value. In the following code I have a record set which is 6004 in size when it comes round the loop it manages to get to 6006 even though I have put an if statment to trap when it reaches the value++ to reset the progress bar to zero and also hide the panel.

    invoiceRecord = (SageDataObject240.InvoiceRecord)_workSpace.CreateObject(Resources.InvoiceRecord);


    pnlProgress.Visible = true;
    progBarSelectInvoices.Value = 0;
    progBarSelectInvoices.Maximum = invoiceRecord.Count;

    List<EdiInvoice> selectedInvoices = new List<EdiInvoice>();
    EdiInvoice invoice;

    DateTime fromDate = chkEnableFromDatePicker.Checked ? dtpFrom.Value.Date : DateTime.MinValue;
    DateTime toDate = chkEnableToDatePicker.Checked ? dtpTo.Value.Date : DateTime.MaxValue;
    int invoiceCount = 0;
    int progressCount = 0;
  int progresbarValue = 0;          
    int maxCount = invoiceRecord.Count + 1;
    while (invoiceRecord.MoveLast())
    {
        progresbarValue = progBarSelectInvoices.Value++;
        bool isPosted = (SDOHelper.Read<sbyte>(invoiceRecord, Resources.POSTED_CODE) == 1);

        if (isPosted)
        {
            int invoiceNo = SDOHelper.Read<int>(invoiceRecord, Resources.INVOICE_NUMBER);
            string invoiceCustomerReference = SDOHelper.Read<string>(invoiceRecord, Resources.ACCOUNT_REF);
            bool isValidCustomerReference = (invoiceCustomerReference == _selectedCustomer.Reference || _selectedCustomer.IncludeBranchInvoices && _selectedCustomer.BranchCodes.ContainsKey(invoiceCustomerReference));

            sbyte invoiceTypeCode = SDOHelper.Read<sbyte>(invoiceRecord, Resources.INVOICE_TYPE_CODE);
            bool isValidType = invoiceTypeCode >= 0 && invoiceTypeCode <= 5;

            string notes1 = SDOHelper.Read<string>(invoiceRecord, "NOTES_1");
            bool isExported = notes1.Length > 2 && notes1.Substring(0, 3).Equals("EDI", StringComparison.CurrentCultureIgnoreCase);

            DateTime invoiceDate = SDOHelper.Read<DateTime>(invoiceRecord, "INVOICE_DATE");
            bool isInDateRange = invoiceDate >= fromDate && invoiceDate <= toDate;

            if (isValidCustomerReference && isValidType && (!isExported || chkIncludeAlreadyExportedInvoices.Checked) && isInDateRange)
            {
                invoice = new EdiInvoice();
                invoice.Customer = string.Format("({0}), {1}", invoiceCustomerReference, SDOHelper.Read<string>(invoiceRecord, Resources.NAME));
                invoice.InvoiceNumber = invoiceNo;
                invoice.Date = SDOHelper.Read<DateTime>(invoiceRecord, "INVOICE_DATE").ToString("dd/MM/yyyy");
                invoice.Type = GetInvoiceOrCredit(invoiceTypeCode);
                invoice.DeliveryAddress = SDOHelper.Read<string>(invoiceRecord, Resources.DEL_ADDRESS_1);
                invoice.Nett = SDOHelper.Read<double>(invoiceRecord, Resources.BASE_TOT_NET) + SDOHelper.Read<double>(invoiceRecord, Resources.BASE_CARR_NET);
                invoice.Vat = SDOHelper.Read<double>(invoiceRecord, Resources.BASE_TOT_TAX) + SDOHelper.Read<double>(invoiceRecord, Resources.BASE_CARR_TAX);

                selectedInvoices.Add(invoice);
            }
        }
           invoiceCount = invoiceRecord.Count;
           progressCount = progBarSelectInvoices.Value;
        if (progressCount++ == maxCount || progressCount==invoiceCount )
        {
            progBarSelectInvoices.Value = 0;
            progressCount = 0;
            pnlProgress.Visible = false;
            Application.DoEvents();
        }
        else

        {
            progBarSelectInvoices.Value++;
            progressCount= progBarSelectInvoices.Value++;
        }
        Application.DoEvents();

Answer 1

Hopefully this will help you to figure this out if we walk through what is happening here. Let's examine what happens when you are entering the 6003rd loop. I will assume that progBarSelectInvoices.Value == 6002 at this point:

What does this line do?

progresbarValue = progBarSelectInvoices.Value++;

At the end of it. progresbarValue == 6004 and progBarSelectInvoices == 6003

progresbarValue is never used again so it could probably just be dumped, but not sure

Then we skip down and do a bunch of other things and hit this line:

progressCount = progBarSelectInvoices.Value;

Here progressCount == 6003 and then we do this:

if (progressCount++ == maxCount || progressCount==invoiceCount)

So what happens here, if maxCount == 6004 than the first part of this if is false BUT now progessCount == 6004. We know that if maxCount is 6004, then invoiceCount == 6003 (from this line maxCount = invoiceRecord.Count + 1;), but now progressCount == 6004 so this is also false. This means we execute the else part.

progBarSelectInvoices.Value++;
progressCount= progBarSelectInvoices.Value++;

What does this do? Well, in the first line we increment progBarSelectInvoices.Value so that is now 6004. Then we move to the second line where we set progressCount to 6004, but then we increment the value of the progress bar again, so it is now 6005. Then we go back to the top of the loop and the first thing we do is increment the progress bar again, which brings us to 6006.

Personally, I try to avoid doing either pre or post increment statements in if statements as it can make the code hard to read. In this case, I would definitely advise against it.

How to fix
One way to fix this - if you know for sure you only have 6004 records and so you will only execute that while loop 6004 times, then just do this (note, I am recommending that you move the increment to the end of the while loop so that you are actually done the work before indicating progress)

while (invoiceRecord.MoveLast())
{
    // All your other code here
    progBarSelectInvoices.Value++;

If you want to be sure you don't exceed the max value you could just add a quick check

while (invoiceRecord.MoveLast())
{
    // All your other code here
    if (progBarSelectInvoices.Value < progBarSelectInvoices.Maximum)
        progBarSelectInvoices.Value++;

You can then do away with the if/else business at the end of the loop that is checking the maxCount and either resetting the progress bar or incrementing it.

Side note
From the presence of the line Application.DoEvents() and the fact that you are not getting an exception when incrementing the progress bar value, I get the sneaky suspicion that you are executing this loop in the UI thread. Whenever you find yourself needing to add Application.DoEvents() in your event handler code, it is a very good time to ask yourself "How can I move this work out of the UI thread?" Calling Application.DoEvents() is almost never a good idea. See here and here

You should move that code into a background thread. The only thing to keep in mind is that you would need to use an invoke call to update the progress bar value.