Each button click incriments the times the background worker runs to copy files

Question

I have a windows application (C#) where I'm copying some files from a directory (recursively) to a drive the users choose from the UI.

It works great - all files move to the appropriate drive.

But the process increments with the amount of times a button is clicked.

I.E.

user clicks 'backup desktop' and it runs to 100% completion
user then clicks 'backup favorites' and it runs to 100% completion, twice.
user then clicks any other button and it runs to 100% completion, three times.

I'm unable to locate where in my following code it's getting called to increment the number of times the process is completed.

I'm sure I'm missing it somewhere - I just don't know where.

I'm using a progress bar so I can let users know graphically what's happening - it runs as intended.

/*
 * ***** *
 * Copy all directories and files recursively
 * ***** *
*/
public void CopyFilesRecursively(DirectoryInfo source, DirectoryInfo target)
{
    foreach (DirectoryInfo dir in source.GetDirectories())
    {
        if ((dir.Name == "My Music") || (dir.Name == "My Pictures") || (dir.Name == "My Videos"))
        {
            //do nothing with it since it is just a system link we cannot access
        }
        else
        {
            CopyFilesRecursively(dir, target.CreateSubdirectory(dir.Name));
        }
    }
    foreach (FileInfo file in source.GetFiles())
        file.CopyTo(Path.Combine(target.FullName, file.Name), true);
}

/* 
 * *****
 * PROGRESS BAR METHODS *
 * *****
*/
void bgWorker_DoWork(object sender, DoWorkEventArgs e)
{
    string buttonSender = clickedButton; //Desktop, Documents, etc.
    switch (buttonSender) {
        case "Desktop":
            destinationLocation = selectedDrive + "Backups-" + DateTime.Now.Month.ToString() + "-" + DateTime.Now.Year.ToString() + "\\Desktop\\";
            string desktopFolder = Environment.GetFolderPath(Environment.SpecialFolder.DesktopDirectory);
            source = new DirectoryInfo(desktopFolder);
            break;
        case "Documents":
            destinationLocation = selectedDrive + "Backups-" + DateTime.Now.Month.ToString() + "-" + DateTime.Now.Year.ToString() + "\\Documents\\";
            string documentsFolder = Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments);
            source = new DirectoryInfo(documentsFolder);
            break;
        case "Favorites":
            destinationLocation = selectedDrive + "Backups-" + DateTime.Now.Month.ToString() + "-" + DateTime.Now.Year.ToString() + "\\Favorites\\";
            string favoritesFolder = Environment.GetFolderPath(Environment.SpecialFolder.Favorites);
            source = new DirectoryInfo(favoritesFolder);
            break;
        default :

            break;
    }

    DirectoryInfo target = new DirectoryInfo(destinationLocation);
    fileCount = source.GetFiles("*", SearchOption.AllDirectories).Length;
    totalFileCount = fileCount;
    int total = totalFileCount; //some number (this is your variable to change)!!
    for (int i = 0; i <= total; i++) //some number (total)
    {
        System.Threading.Thread.Sleep(100);
        int percents = (i * 100) / total;
        bgWorker.ReportProgress(percents, i);
        //2 arguments:
        //1. procenteges (from 0 t0 100) - i do a calcumation 
        //2. some current value!
    }
    if (!Directory.Exists(destinationLocation))
    {
        Directory.CreateDirectory(destinationLocation);
    }
    CopyFilesRecursively(source, target);
}

void bgWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    essentialItemsProgressBar.Visible = true;
    essentialItemsProgressBar.Value = e.ProgressPercentage;

    int percent = (int)(((double)(essentialItemsProgressBar.Value - essentialItemsProgressBar.Minimum) /
    (double)(essentialItemsProgressBar.Maximum - essentialItemsProgressBar.Minimum)) * 100);
    using (Graphics gr = essentialItemsProgressBar.CreateGraphics())
    {
        gr.DrawString(percent.ToString() + "%",
            SystemFonts.DefaultFont,
            Brushes.Black,
            new PointF(essentialItemsProgressBar.Width / 2 - (gr.MeasureString(percent.ToString() + "%",
                SystemFonts.DefaultFont).Width / 2.0F),
            essentialItemsProgressBar.Height / 2 - (gr.MeasureString(percent.ToString() + "%",
                SystemFonts.DefaultFont).Height / 2.0F)));
    }

    essentialItemsProgressLabel.Visible = true;
    essentialItemsProgressLabel.Text = String.Format("Progress: {0} % - All {1} Files Transferred", e.ProgressPercentage, clickedButton);
    //essentialItemsProgressLabel.Text = String.Format("Total items transfered: {0}", e.UserState);
}

void bgWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    //do the code when bgv completes its work
    essentialItemsProgressBar.Visible = false;
}
/* 
 * *****
 * END OF PROGRESS BAR METHODS *
 * *****
*/
/*
 * ***** DESKTOP BACKUP ***** *
*/
private void backupDesktopButton_Click(object sender, EventArgs e)
{
    clickedButton = ((Button)sender).Text.ToString();
    selectedDrive = backupDriveCombo.SelectedItem.ToString();
    bgWorker.DoWork += new DoWorkEventHandler(bgWorker_DoWork);
    bgWorker.ProgressChanged += new ProgressChangedEventHandler(bgWorker_ProgressChanged);
    bgWorker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(bgWorker_RunWorkerCompleted);
    bgWorker.WorkerReportsProgress = true;
    bgWorker.RunWorkerAsync();
}
/*
 * ***** DOCUMENTS BACKUP ***** *
*/
private void backupDocumentsButton_Click(object sender, EventArgs e)
{
    clickedButton = ((Button)sender).Text.ToString();
    selectedDrive = backupDriveCombo.SelectedItem.ToString();
    bgWorker.DoWork += new DoWorkEventHandler(bgWorker_DoWork);
    bgWorker.ProgressChanged += new ProgressChangedEventHandler(bgWorker_ProgressChanged);
    bgWorker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(bgWorker_RunWorkerCompleted);
    bgWorker.WorkerReportsProgress = true;
    bgWorker.RunWorkerAsync();
}

.. because you are **adding** event handlers to `DoWork` in each click? — stuartd, Nov 01 '16 at 17:30
_"How would I better handle that?"_ -- just don't do that. Either configure your `BackgroundWorker` once (if you add it as a component to your form, you can set the event handlers using the property window in the Designer), or create a new `BackgroundWorker` every time you want to run it. But whatever you do, don't just keeping adding a new handler to the same event on the same object every time you run the worker. — Peter Duniho, Nov 01 '16 at 17:34
See marked duplicate for a description of and solution for the exact problem. See also posts like https://stackoverflow.com/questions/6200424/event-fires-more-and-more-times, https://stackoverflow.com/questions/399648/preventing-same-event-handler-assignment-multiple-times, https://stackoverflow.com/questions/25190270/c-sharp-event-handler-is-called-multiple-times-when-event-is-raised-once, and others [here](https://stackoverflow.com/search?tab=relevance&q=%5bc%23%5d%20event%20handler%20called%20multiple%20times) — Peter Duniho, Nov 01 '16 at 17:39

score 3 · Accepted Answer · edited Nov 01 '16 at 17:51

The problem is in this line(in backupDocumentsButton_Click() method):

bgWorker.DoWork += new DoWorkEventHandler(bgWorker_DoWork);

You should do this only once(i.e in creating bgWorker), but you do this on every button click. After first click you add one handler to the button, after second click you add one more - 2 times it will be called...

You should change your code:

private void backupDocumentsButton_Click(object sender, EventArgs e)
{
    clickedButton = ((Button)sender).Text.ToString();
    selectedDrive = backupDriveCombo.SelectedItem.ToString();
    // without these lines
    // bgWorker.DoWork += new DoWorkEventHandler(bgWorker_DoWork); 
    // bgWorker.ProgressChanged += new ProgressChangedEventHandler(bgWorker_ProgressChanged);
    // bgWorker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(bgWorker_RunWorkerCompleted);
    bgWorker.WorkerReportsProgress = true;
    bgWorker.RunWorkerAsync();
}

And you should add line after creating bgWorker, ie:

BackgroundWorker bgWorker = new BackgroundWorker();
bgWorker.DoWork += bgWorker_DoWork;
bgWorker.ProgressChanged += bgWorker_ProgressChanged;
bgWorker.RunWorkerCompleted += bgWorker_RunWorkerCompleted;

You should only add event handlers one time.

They said the same in the comments - but this clarified it perfectly in a way I could easily see/understand. Thank you! This worked great. I will accept in a few minutes when I can. — Hanny, Nov 01 '16 at 17:40

Each button click incriments the times the background worker runs to copy files

1 Answers1