patterncsharpMinor

Recursive directory copy program

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

recursiveprogramcopydirectory

Problem

A little while ago, I had to write a little C# application to recover my HDD data (full context on this question)

To answer my problem I developed a console application which job was to recursively copy the entire folder tree of my HDD.

The code can be seen below :

```
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace CopyCat
{
class Program
{
private static long _fileCount = 0;
private static long _byteCount = 0;
private static long _byteProgress = 0;
private static ProgressBar _progressCount = new ProgressBar();

static void Main(string[] args)
{
Directory.Exists(args[0]);
Directory.Exists(args[1]);

FileDiscovery(args[0]);
FileCopy(args[0], args[1]);

Console.ReadLine();
}

static void FileCopy(String source, String dest)
{
try
{
foreach (var file in Directory.EnumerateFiles(source, "*", SearchOption.TopDirectoryOnly))
{
try
{
if (file == null) continue;

var oFile = File.OpenRead(file);
var dFile = File.Open(Path.Combine(dest, Path.GetFileName(file)), FileMode.Create,
FileAccess.ReadWrite);
oFile.CopyTo(dFile, 104857600);

oFile.Close();

dFile.Flush();
dFile.Close();

_byteProgress += new FileInfo(file).Length;

_progressCount.Report((double)_byteProgress / (double)_byteCount);
}
catch (Exception e)
{
Console.WriteLine("[COPY][ERROR] : Couldn't copy file : {0} => {1}", file, e.Message);

Solution

This

Directory.Exists(args[0]);
Directory.Exists(args[1]);

doesn't serve any purpose because you aren't evaluating the returned boolean value. In fact its more dangerous to use it with the args not checked for null at all. Your application will just crash if the application is called using only one or no argument at all.

The static void FileDiscovery() method would benefit from a better name like CalculateDirectorySize() and by returning a long instead of void.

Another point to mention is that the DirectoryInfo class contains the method GetFileSystemInfos() which would lead to the following change

private static long CalculateDirectorySize(String dir)
    {
        long directorySize = 0;
        var dirInfo = new DirectoryInfo(dir);
        try
        {
            foreach (var fileInfo in dirInfo.GetFileSystemInfos("*", SearchOption.TopDirectoryOnly))
            {
                directorySize += fileInfo.Length;
            }

            foreach (var directory in Directory.EnumerateDirectories(dir, "*", SearchOption.TopDirectoryOnly))
            {
               directorySize += CalculateDirectorySize(directory);
            }
        }
        catch (Exception exception)
        {
            Console.WriteLine("[DISCOVERY][WARNING] : Couldn't open directory : {0}", dir);
        }
        return directorySize;
    }

I have added private access modifier to the method, because it is a good habit to add one.

Because _fileCount is no where used I have removed it.

The FileCopy() method is using a strange way to copy the files by reading and writing them using FileStream's.

-
A more idiomatic way would be to use one of the overloaded File.Copy() methods. This methods are doing under the hood the same like your code but the streams are properly closed if any exception occurs.

-
Directory.CreateDirectory() can be called regardless if the directory exists or not. You can just skip the check for Exists().

Having three times Path.Combine(dest, dir) won't be necessary if the result is stored in a variable.

Instead of calling GetFileName() for a directory you should call GetDirectoryName()

this would result in the following change

private static void FileCopy(String source, String dest)
    {
        try
        {
            foreach (var file in Directory.EnumerateFiles(source, "*", SearchOption.TopDirectoryOnly))
            {
                try
                {
                    File.Copy(file, Path.Combine(dest, Path.GetFileName(file)));

                    _byteProgress += new FileInfo(file).Length;

                    _progressCount.Report((double)_byteProgress / (double)_byteCount);
                }
                catch (Exception e)
                {
                    Console.WriteLine("[COPY][ERROR] : Couldn't copy file  : {0} => {1}", file, e.Message);
                }
            }
            foreach (var directory in Directory.EnumerateDirectories(source, "*", SearchOption.TopDirectoryOnly))
            {
                if (directory == @"G:\$RECYCLE.BIN") continue;

                var dir = Path.GetDirectoryName(directory);
                var destination = Path.Combine(dest, dir)

                Directory.CreateDirectory(destination);

                FileCopy(directory, destination);
            }
        }
        catch (Exception exception)
        {
            Console.WriteLine("[COPY][WARNING] : Couldn't open directory  : {0}", source);
        }
    }

Based on the comment

I was aware of the File.Copy() method at the time of writing the application BUT (and that's why I included the context) this method was throwing the same error as my explorer when used : that the file was not accessible and/or corrupted.

I would like to suggest to only use this kind of copy operation if the File.Copy() method throws an IOException having its own method like so

private static readonly int blockSize = 104857600;  
private static bool SafeFileCopy(string source, string destination) {

    try
    {
        using(var sourceStream = File.OpenRead(source))
        using(var destinationStream = File.Open(destination, FileMode.Create,
                            FileAccess.ReadWrite))
        {
            sourceStream.CopyTo(destinationStream, blockSize);
            return true
        }

    }
    catch (Exception ex)
    {
        // do some logging
    }
    return false; 

}

Resulting in the former foreach like so

```
foreach (var file in Directory.EnumerateFiles(source, "*", SearchOption.TopDirectoryOnly))
{
string destinationFile = Path.Combine(dest, Path.GetFileName(file));
try
{
File.Copy(file, destinationFile);

_byteProgress += new FileInfo(file).Length;

_progressCount.Report((double)_byteProgress / (double)_byteCount);

Code Snippets

Directory.Exists(args[0]);
Directory.Exists(args[1]);

private static long CalculateDirectorySize(String dir)
    {
        long directorySize = 0;
        var dirInfo = new DirectoryInfo(dir);
        try
        {
            foreach (var fileInfo in dirInfo.GetFileSystemInfos("*", SearchOption.TopDirectoryOnly))
            {
                directorySize += fileInfo.Length;
            }

            foreach (var directory in Directory.EnumerateDirectories(dir, "*", SearchOption.TopDirectoryOnly))
            {
               directorySize += CalculateDirectorySize(directory);
            }
        }
        catch (Exception exception)
        {
            Console.WriteLine("[DISCOVERY][WARNING] : Couldn't open directory : {0}", dir);
        }
        return directorySize;
    }

private static void FileCopy(String source, String dest)
    {
        try
        {
            foreach (var file in Directory.EnumerateFiles(source, "*", SearchOption.TopDirectoryOnly))
            {
                try
                {
                    File.Copy(file, Path.Combine(dest, Path.GetFileName(file)));

                    _byteProgress += new FileInfo(file).Length;

                    _progressCount.Report((double)_byteProgress / (double)_byteCount);
                }
                catch (Exception e)
                {
                    Console.WriteLine("[COPY][ERROR] : Couldn't copy file  : {0} => {1}", file, e.Message);
                }
            }
            foreach (var directory in Directory.EnumerateDirectories(source, "*", SearchOption.TopDirectoryOnly))
            {
                if (directory == @"G:\$RECYCLE.BIN") continue;

                var dir = Path.GetDirectoryName(directory);
                var destination = Path.Combine(dest, dir)

                Directory.CreateDirectory(destination);

                FileCopy(directory, destination);
            }
        }
        catch (Exception exception)
        {
            Console.WriteLine("[COPY][WARNING] : Couldn't open directory  : {0}", source);
        }
    }

private static readonly int blockSize = 104857600;  
private static bool SafeFileCopy(string source, string destination) {

    try
    {
        using(var sourceStream = File.OpenRead(source))
        using(var destinationStream = File.Open(destination, FileMode.Create,
                            FileAccess.ReadWrite))
        {
            sourceStream.CopyTo(destinationStream, blockSize);
            return true
        }

    }
    catch (Exception ex)
    {
        // do some logging
    }
    return false; 

}

foreach (var file in Directory.EnumerateFiles(source, "*", SearchOption.TopDirectoryOnly))
            {
                string destinationFile = Path.Combine(dest, Path.GetFileName(file));
                try
                {
                    File.Copy(file, destinationFile);

                    _byteProgress += new FileInfo(file).Length;

                    _progressCount.Report((double)_byteProgress / (double)_byteCount);
                }
                catch (IOException ioex)
                {
                    if (!SafeFileCopy(file, destinationFile))
                    {
                         // do some logging here
                    }
                }
                catch (Exception e)
                {
                    Console.WriteLine("[COPY][ERROR] : Couldn't copy file  : {0} => {1}", file, e.Message);
                }
            }

Context

StackExchange Code Review Q#122241, answer score: 3

Revisions (0)

No revisions yet.