Async file copy/move

Question

My effort to write async methods for copy/move a file in C#

public static class FileHelper
{
 private const int _FileStreamDefaultBufferSize = 4096;
 
 private static bool HasNetworkDrive(string path)
 {
 try
 {
 return new DriveInfo(path).DriveType == DriveType.Network;
 }
 catch (Exception)
 {
 return false;
 }
 }
 private static bool IsUncPath(string path)
 {
 try
 {
 return new Uri(path).IsUnc;
 }
 catch (Exception)
 {
 return false;
 }
 }
 
 private static async Task InternalCopyToAsync(string sourceFilePath, string destFilePath, FileOptions? sourceFileOptions = null, bool overwrite = false)
 {
 sourceFilePath.AssertHasText(nameof(sourceFilePath));
 destFilePath.AssertHasText(nameof(destFilePath));
 var sourceStreamFileOpt = (sourceFileOptions ?? FileOptions.SequentialScan) | FileOptions.Asynchronous;
 using (FileStream sourceStream = new FileStream(sourceFilePath, FileMode.Open, FileAccess.Read, FileShare.Read, _FileStreamDefaultBufferSize, sourceStreamFileOpt))
 using (FileStream destinationStream = new FileStream(destFilePath, overwrite ? FileMode.Create : FileMode.CreateNew, FileAccess.Write, FileShare.None, _FileStreamDefaultBufferSize, true))
 {
 await sourceStream.CopyToAsync(destinationStream, _FileStreamDefaultBufferSize).ConfigureAwait(false);
 }
 }
 
 public static async Task MoveAsync(string sourceFilePath, string destFilePath)
 {
 sourceFilePath.AssertHasText(nameof(sourceFilePath));
 destFilePath.AssertHasText(nameof(destFilePath));
 if (IsUncPath(sourceFilePath) || HasNetworkDrive(sourceFilePath) || IsUncPath(destFilePath) || HasNetworkDrive(destFilePath))
 {
 await InternalCopyToAsync(sourceFilePath, destFilePath, FileOptions.DeleteOnClose).ConfigureAwait(false);
 return;
 }
 FileInfo sourceFileInfo = new FileInfo(sourceFilePath);
 string sourceDrive = Path.GetPathRoot(sourceFileInfo.FullName);
 FileInfo destFileInfo = new FileInfo(destFilePath);
 string destDrive = Path.GetPathRoot(destFileInfo.FullName);
 if (sourceDrive == destDrive)
 {
 File.Move(sourceFilePath, destFilePath);
 return;
 }
 await Task.Run(() => File.Move(sourceFilePath, destFilePath)).ConfigureAwait(false);
 }
 
 public static async Task CopyAsync(string sourceFileName, string destFileName)
 {
 await InternalCopyToAsync(sourceFileName, destFileName).ConfigureAwait(false);
 }
 public static async Task CopyAsync(string sourceFileName, string destFileName, bool overwrite)
 {
 await InternalCopyToAsync(sourceFileName, destFileName, overwrite: overwrite).ConfigureAwait(false);
 }
}

The extension method AssertHasText is just throwing an ArgumentNullException if !string.IsNullOrWhiteSpace(argument)

Regarding the implementation of MoveAsync I followed these guidelines

• If the source or dest path are a from a network drive, or UNC path, I call the internal async copy method with the flag FileOptions.DeleteOnClose
• If the source drive is the same as the dest drive, I call the standard File.Move method, because it is an almost-instantaneous operation, as the headers are changed but the file contents are not moved
• In any other case, I use a Task with the standard File.Move. I differentiate the above case to save an unnecessary thread

Question: Regarding the implementation of CopyAsync it will always copy the stream, Can previous claims be applied to the copy as well?

EDIT: Adding the implementation of AssertArgumentHasText

public static void AssertArgumentHasText(this string argument, string name)
{
 if (argument.IsNullOrEmpty())
 {
 throw new ArgumentNullException(
 name,
 string.Format(
 CultureInfo.InvariantCulture,
 "Argument '{0}' cannot be null or resolve to an empty string : '{1}'.", name, argument));
 }
}

• Phate01: @HenrikHansen as I already stated, it is a wrapper for string.IsNullOrWhiteSpace and throws an ArgumentNullException taking the name of the var to pass to the exception as an argument.
• Phate01: @HenrikHansen this is really not the point of the question, however that's an extension method and has actually 2 args, so I'll update the question with the implementation and we can move on to real business
• Peter Csala: From what perspective do you want the code to be reviewed? Or is there any particular question?
• Phate01: @PeterCsala actually yes, my concerns are about the CopyAsync method, I updated my question
• Peter Csala: Sorry, but I don't get your question. You are calling the CopyAsync only in case of Network drive, in other cases you call File.Move. So, why do you want to apply the same branching for the Copy?

Answer 1

Regarding the implementation of CopyAsync it will always copy the stream.
Can previous claims be applied to the copy as well?

Your current implementation exposes two sort of operations:

1. Move
2. Copy

They are sharing the same signature (more or less). The override functionality is not controllable in case of Move from the consumer perspective.

The Copy operation is optimized to reduce latency (to take advantage of data locality) by branching based on the location of the drive. The same branching could be applied to the Move as well to provide symmetric behaviour. If it branches in the same way then extension in any direction (new driver location (for example Kubernetes virtual drive), new operation (for example Delete), etc.) would be way more convenient.

From readability and maintainability perspective it is easier to have symmetric functions, because after awhile (without any context / reasoning why Move does not apply the same optimization as Copy do) no one will know who and why did this.

Answer 2

Errors aren't answers

This:

 try
 {
 return new DriveInfo(path).DriveType == DriveType.Network;
 }
 catch (Exception)
 {
 return false;
 }

will harm and not help you. What if a caller passes the integer 3 for path? The network drive status is certainly not True, but you can't say that it's False, either. It's an error, and should be allowed to fall through, which means you should not be catching Exception. In a different vein: what if (for some weird reason) the DriveType property lookup runs into an OutOfMemoryException? That is also not proof that this is not a network drive.

If you understand that there are certain (perhaps IOException or derived) exceptions that actually do indicate that this is not a network drive, then catch those specifically.

• Phate01: Good point, it is a further edit to be done. What about the main question instead?