trjhtr

I have a simple WinForm application that copies all files and directories from three folders into a fourth one. All folders are chosen/entered by the user.
Security is not a concern.

In order to prevent the application from visibly hanging I added a BackgroundWorker that updates the values of a ProgressBar. However since the task of creating directory structure and copying files isn't all that simple I fear I might have written a poor cancel handling.

Secondary concern - I might have gone overboard with the exception handling.

Apart from that ANY feedback, review or suggestion for improvement is greatly appreciated!

Some background:

The ProgressBar is named "pbMain"

The three TextBoxes that are the origin of the copy operation are named "tbECheckPath", "tbCorePath" and "tbServicePath"

There is a List<TextBox> DebugTextBoxes variable that holds all the three mentioned TextBoxes.

GitHub of the project.
The code in Pastebin

The code:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.IO;
using System.Threading;
using System.Windows.Forms;

namespace PublishForQA

 public partial class FormProgressBar : Form
 
 static FormPublisher formPublisher = (FormPublisher)Form.ActiveForm;
 static FormProgressBar formProgressBar;
 static List<TextBox> debugTextBoxes = formPublisher.DebugTextBoxesList;
 static BackgroundWorker backgroundWorker = new BackgroundWorker();
 static DoWorkEventArgs WorkArgs;
 static int TotalOperationsCount;
 static int CurrentOpeartionCount;

 public FormProgressBar()
 
 TotalOperationsCount = 0;
 CurrentOpeartionCount = 0;
 InitializeComponent();
 formProgressBar = this;
 backgroundWorker.DoWork += backgroundWorker_DoWork;
 backgroundWorker.ProgressChanged += backgroundWorker_ProgressChanged;
 backgroundWorker.RunWorkerCompleted += backgroundWorker_RunWorkerCompleted;
 backgroundWorker.WorkerReportsProgress = true;
 backgroundWorker.WorkerSupportsCancellation = true;
 backgroundWorker.RunWorkerAsync();
 

 private void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
 
 WorkArgs = e;
 pbMain.Maximum = GetOperationsCount();
 CopyFilesAndDirectories();
 

 private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e)
 
 pbMain.Value = e.ProgressPercentage;
 

 private void backgroundWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
 
 if (e.Error != null)
 
 MessageBox.Show("An error occurred during copying:" + Environment.NewLine + e.Error.Message, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
 Cleanup();
 
 else if (e.Cancelled)
 
 MessageBox.Show("Copy operation aborted.", "Abort", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
 Cleanup();
 
 else
 
 MessageBox.Show("Copy operation completed successfully!", "Operation success", MessageBoxButtons.OK, MessageBoxIcon.Information);
 Cleanup();
 
 

 private void btnCancel_Click(object sender, EventArgs e)
 
 backgroundWorker.CancelAsync();
 

 /// <summary>
 /// Checks if "CancellationPending" is true and if it is aborts the thread and sets
 /// the DoWorkEventArgs's Cancel property to true.
 /// </summary>
 static void CheckForCancel()
 
 if (backgroundWorker.CancellationPending)
 
 WorkArgs.Cancel = true;
 Thread.CurrentThread.Abort();
 
 

 /// <summary>
 /// Marks the BackgroundWorker to be disposed and closes the progress bar form.
 /// </summary>
 static void Cleanup()
 
 backgroundWorker.Dispose();
 formProgressBar.Close();
 

 /// <summary>
 /// Counts all the directories that need to be created and all the
 /// files that need to be copied from each designated folder.
 /// </summary>
 /// <returns></returns>
 public static int GetOperationsCount()
 
 formProgressBar.lblCurrentOperation.Text = "Counting the total number of operations...";
 foreach (var tb in debugTextBoxes)
 
 CheckForCancel();
 TotalOperationsCount += Directory.GetFiles(tb.Text, "*", SearchOption.AllDirectories).Length;
 CheckForCancel();
 TotalOperationsCount += Directory.GetDirectories(tb.Text, "*", SearchOption.AllDirectories).Length;
 formProgressBar.lblCurrentPath.Text = tb.Text;
 

 return TotalOperationsCount;
 

 /// <summary>
 /// Recreates the directory structure at the target location and copies all files from the source recursively.
 /// </summary>
 public static void CopyFilesAndDirectories()
 
 foreach (var tb in debugTextBoxes)
 
 CheckForCancel();
 //If there is a task name provided we add a backslash, otherwise the QA Folder path's
 //last backslash will suffice.
 string destinationPath = formPublisher.tbQAFolderPath.Text + ((formPublisher.tbTaskName.Text.Length > 0) ? formPublisher.tbTaskName.Text + "\" : string.Empty);

 //We set the name of the destination folder, depending
 //on the TextBox we are iterating over.
 switch (tb.Name)
 
 case "tbECheckPath":
 destinationPath += "E-Check\";
 break;
 case "tbCorePath":
 destinationPath += "E-CheckCore\";
 break;
 case "tbServicePath":
 destinationPath += "E-CheckService\";
 break;
 default:
 break;
 

 if (!CreateDirectoryStructure(tb.Text, destinationPath)) return;
 if (!CopyFiles(tb.Text, destinationPath)) return;
 
 

 /// <summary>
 /// Gets all the directories in a target path and recreates the same
 /// directory structure in the destination path.
 /// </summary>
 /// <param name="sourcePath">The path from which to read the directory structure.</param>
 /// <param name="destinationPath">The path where to recreate the directory structure.</param>
 /// <returns>"True" if the operation was successful, "false" if an exception was raised.</returns>
 public static bool CreateDirectoryStructure(string sourcePath, string destinationPath)
 
 formProgressBar.lblCurrentOperation.Text = "Creating directory structure...";
 //These variables will hold the current source and target path of the "for" iteration.
 //They will be used to show more information in the exception catching.
 string sourceDir = FormPublisher.ErrorBeforeDirectoryLoop;
 string targetDir = FormPublisher.ErrorBeforeDirectoryLoop;
 try
 
 //First we create the directory structure.
 Directory.CreateDirectory(destinationPath);
 foreach (string dirPath in Directory.GetDirectories(sourcePath, "*", SearchOption.AllDirectories))
 
 CheckForCancel();
 sourceDir = dirPath;
 targetDir = dirPath.Replace(sourcePath, destinationPath);
 formProgressBar.lblCurrentPath.Text = targetDir;
 Directory.CreateDirectory(targetDir);
 CurrentOpeartionCount++;
 backgroundWorker.ReportProgress(CurrentOpeartionCount);
 
 return true;
 
 #region catch block
 catch (UnauthorizedAccessException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("The caller does not have the required permission for "" + targetDir + "".", sourceDir, targetDir, ex), "Unauthorized Access Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (ArgumentNullException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("The path passed for directory creation is null.", sourceDir, targetDir, ex), "Argument Null Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (ArgumentException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("The path passed for directory creation is invalid.", sourceDir, targetDir, ex), "Argument Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (PathTooLongException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("Cannot create target directory, path is too long.", sourceDir, targetDir, ex), "Path Too Long Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (DirectoryNotFoundException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("The path passed for directory creation could not be found.", sourceDir, targetDir, ex), "Directory Not Found Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (IOException ex)
 
 //IO Exception can be either the passed path is a file or the network name is not known.
 //Since we have previous checks in place to make sure the path is a directory,
 //the second possible error is shown.
 MessageBox.Show(ExceptionMessageBuilder.Directory("The network name is not known.", sourceDir, targetDir, ex), "IO Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (NotSupportedException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("The path passed contains an illegal colon character.", sourceDir, targetDir, ex), "Not Supported Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (ThreadAbortException)
 
 Thread.ResetAbort();
 return false;
 
 catch (Exception ex)
 
 MessageBox.Show(ExceptionMessageBuilder.Directory("Unexpected exception occurred:" + Environment.NewLine + ex.Message, sourceDir, targetDir, ex), "Unexpected Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 #endregion
 

 /// <summary>
 /// Copies all files from the target path to the destination path,
 /// overriding any existing ones.
 /// </summary>
 /// <param name="sourcePath">The path from which to copy all the files.</param>
 /// <param name="destinationPath">The path where to copy all the files.</param>
 /// <returns>"True" if the operation was successful, "false" if an exception was raised.</returns>
 public static bool CopyFiles(string sourcePath, string destinationPath)
 
 formProgressBar.lblCurrentOperation.Text = "Copying files...";
 //These variables will hold the current source and target path of the "for" iteration.
 //They will be used to show more information in the exception catching.
 //But first they are set to the string used to indicate an error before the loop.
 string sourceFile = FormPublisher.ErrorBeforeFileLoop;
 string targetFileDir = FormPublisher.ErrorBeforeFileLoop;
 try
 
 //We copy all files, overwriting any existing ones.
 foreach (string filePath in Directory.GetFiles(sourcePath, "*", SearchOption.AllDirectories))
 
 CheckForCancel();
 sourceFile = filePath;
 targetFileDir = filePath.Replace(sourcePath, destinationPath);
 formProgressBar.lblCurrentPath.Text = filePath;
 File.Copy(filePath, targetFileDir, true);
 CurrentOpeartionCount++;
 backgroundWorker.ReportProgress(CurrentOpeartionCount);
 
 return true;
 
 #region catch block
 catch (UnauthorizedAccessException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("The caller does not have the required permission for "" + targetFileDir + "".", sourceFile, targetFileDir, ex), "Unauthorized Access Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (ArgumentNullException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("Either the source or destination file paths are null.", sourceFile, targetFileDir, ex), "Argument Null Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (ArgumentException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("Either the source or destination file paths are invalid.", sourceFile, targetFileDir, ex), "Argument Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (PathTooLongException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("Either the source or destination file paths are too long.", sourceFile, targetFileDir, ex), "Path Too Long Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (DirectoryNotFoundException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("Either the source or destination file paths could not be found.", sourceFile, targetFileDir, ex), "Directory Not Found Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (NotSupportedException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("Either the source or destination file paths are invalid.", sourceFile, targetFileDir, ex), "Not Supported Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (FileNotFoundException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File(""" + sourceFile + "" was not found.", sourceFile, targetFileDir, ex), "File Not Found Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (IOException ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("An I/O error has occurred.", sourceFile, targetFileDir, ex), "IO Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 catch (ThreadAbortException)
 
 Thread.ResetAbort();
 return false;
 
 catch (Exception ex)
 
 MessageBox.Show(ExceptionMessageBuilder.File("An unexpected exception has occurred:" + Environment.NewLine + ex.Message, sourceFile, targetFileDir, ex), "Unexpected Exception", MessageBoxButtons.OK, MessageBoxIcon.Error);
 return false;
 
 #endregion
 
 

1) The behavior and timing of Thread.Abort method is hard to predict, so you should avoid using it unless you 100% know what you are doing. In my experience, it is only used as last resort measure in rare exotic circumstances. It should not be used as part of regular workflow of your app. In you case, for example, I highly doubt that aborting a thread does anything useful, since File.Copy is a native call, and it probably can't be aborted by CLR.

2) Last time I checked there was no File.Copy method that supports cancellation. But that was a long time ago, maybe things have changed since then. But if it is still the case, there are other options:

• Windows has a native method for copying files that supports cancellation. See: http://www.pinvoke.net/default.aspx/kernel32.copyfileex
  


• You can use streams to copy files in chunks. See: https://stackoverflow.com/a/7680710/1386995
  


• Use a third-party library for asynchronous I/O. There are plenty of those.

3) In general you should try to separate business logic from UI. There should be a different class that does the copying, and your Form should be unaware of its implementation details.

4) Why are you using static fields? Don't, unless you have to for some reason.

5) IMHO, nowadays background worker is rarely used (async programming made it kind of obsolete), but so is WinForms. So apart from your code being somewhat out of date, your use of background worker looks alright to me, I would not call it overly complex.

6) As for exceptions:

• Validate input beforehand where possible, instead of waiting for exceptions to occur. For example, you can easily check whether the file path is null or whether it has correct format before user tries to copy anything. This can drastically reduce the amount of exceptions your have to catch later on.


• Think about who your users are. Do they really need to know what went wrong exactly? Can they do something about it? If so, then yes, the way you handle exceptions is correct. If no, then instead of generating 100500 different message, you might just want to show a single one that would allow user to easily send an underlying exception and stack trace to your emial :)