4
\$\begingroup\$

I have tried to implement a smart copy in C# for Windows CE, my version is somehow not performing better that the File.Copy method it's almost the same performance. Is there a way to make it more efficient?

public class SmartCopy : IDisposable
    {
        private List<CopyEntry> _entries = new List<CopyEntry>();
        private int _refArrayIndex;
        private byte[] _refArray;
        private string _sourceDir;
        private string _destDir;

        private const int BUFFER_SIZE = 1024 * 128;

        private List<string> _lstFailedFiles = new List<string>();

        public SmartCopy(int bufferSize, string sourceDir, string destDir)
        {
            if (bufferSize > 0)
            {
                _refArray = new byte[bufferSize];
                TestLogger.Log(string.Format("buffer length = {0} MB", _refArray.Length / (1024 * 1024)));
            }
            _sourceDir = sourceDir;
            _destDir = destDir;
            if (!Directory.Exists(sourceDir))
            {
                throw new ArgumentException(sourceDir + " does not exist");
            }
        }

        private class CopyEntry
        {
            public int StartIndex { get; set; }
            public int Length { get; set; }
            public string FileName { get; set; }
        }     
        public void DoCopy()
        {
            TestLogger.Log("Start");
            _refArrayIndex = 0;
            _entries.Clear();
            ReadFiles(_sourceDir);
            if (_entries.Count > 0)
            {
                FlushBuffer();
            }
            Dispose();
            TestLogger.Log("Stop");
        }

        private void ReadFiles(string sourceDir)
        {
            foreach (string fileName in Directory.GetFiles(sourceDir))
            {
                FileStream fs = new FileStream(fileName, FileMode.Open);
                if (fs.Length > _refArray.Length)
                {
                    fs.Close();
                    HandleLargeFile(fileName);
                    continue;
                }

                try
                {
                    if ((_refArrayIndex + fs.Length) > _refArray.Length)
                    {
                        FlushBuffer();
                        _refArrayIndex = 0;
                        _entries.Clear();
                    }
                    long bytesToRead = fs.Length;
                    int bytesRead = 0;
                    int index = _refArrayIndex;
                    while (bytesRead < bytesToRead)
                    {
                        bytesRead += fs.Read(_refArray, (index + bytesRead), (int)(bytesToRead - bytesRead));
                    }
                    CopyEntry entry = new CopyEntry
                    {
                        FileName = fileName.Replace(_sourceDir, _destDir),
                        StartIndex = _refArrayIndex,
                        Length = bytesRead
                    };
                    _entries.Add(entry);
                    _refArrayIndex += bytesRead;
                }
                catch
                {
                    _lstFailedFiles.Add(fileName);
                }
                finally
                {
                    if (fs != null)
                    {
                        fs.Close();
                    }
                }
            }
            foreach (string dir in Directory.GetDirectories(sourceDir))
            {
                string dirDest = dir.Replace(_sourceDir, _destDir);
                dirDest.CreateDirectory();
                ReadFiles(dir);
            }
        }

        private void HandleLargeFile(string sourceFileName)
        {
            string destFileName = sourceFileName.Replace(_sourceDir, _destDir);
            string dir = Path.GetDirectoryName(destFileName);
            if (!Directory.Exists(dir))
            {
                Directory.CreateDirectory(dir);
            }

            FileStream fsRead = null;
            FileStream fsWrite = null;

            try
            {
                fsRead = new FileStream(sourceFileName, FileMode.Open);
                int bytesToWrite = (int)fsRead.Length;
                if (bytesToWrite > INTERNAL_BUFFER)
                {
                    fsWrite = File.Create(destFileName, INTERNAL_BUFFER);
                }
                else
                {
                    fsWrite = File.Create(destFileName);
                }
                fsWrite.SetLength(bytesToWrite);

                int bufferSize = BUFFER_SIZE;
                int writeIndex = 0;
                byte[] buffer = new byte[BUFFER_SIZE];
                int bytesRead = 0;

                while (bytesToWrite > 0)
                {
                    bufferSize = Math.Min(bytesToWrite, BUFFER_SIZE);
                    bytesRead = fsRead.Read(buffer, 0, bufferSize);
                    fsWrite.Write(buffer, 0, bytesRead);
                    bytesToWrite -= bytesRead;
                    writeIndex += bytesRead;
                }
            }
            catch { }
            finally
            {
                if (fsRead != null)
                {
                    fsRead.Close();
                }
                if (fsWrite != null)
                {
                    fsWrite.Close();
                }
            }
        }
        private const int INTERNAL_BUFFER = 32 * 1024;
        private void FlushBuffer()
        {
            TestLogger.Log(string.Format("flushing, count = {0}, index = {1}", _entries.Count, _refArrayIndex));

            _entries.ForEach(entry =>
            {
                FileStream fs = null;
                try
                {
                    string dir = Path.GetDirectoryName(entry.FileName);
                    if (!Directory.Exists(dir))
                    {
                        Directory.CreateDirectory(dir);
                    }
                    if (entry.Length > INTERNAL_BUFFER)
                    {
                        fs = File.Create(entry.FileName, INTERNAL_BUFFER);
                    }
                    else
                    {
                        fs = File.Create(entry.FileName);
                    }
                    fs.SetLength(entry.Length);
                    fs.Write(_refArray, entry.StartIndex, entry.Length);
                }
                finally
                {
                    if (fs != null)
                    {
                        fs.Close();
                    }
                }
            });
        }

        public void Dispose()
        {
            _refArray = null;
        }
    }

The calling code is:

SmartCopy sCopy = new SmartCopy(1024*1024*4, sourceDir, destDir);
                        sCopy.DoCopy();
\$\endgroup\$
1
  • \$\begingroup\$ Can you add an example of the project running, if possible? \$\endgroup\$
    – 13aal
    May 22, 2016 at 1:13

1 Answer 1

1
\$\begingroup\$

The using statement

You shouldn't call .Close() or .Dispose() manually the using statement is usually preferred and in your case it will even remove the finally block :

private void ReadFiles(string sourceDir)
{
    foreach (string fileName in Directory.GetFiles(sourceDir))
    {
        FileStream fs = new FileStream(fileName, FileMode.Open);
        if (fs.Length > _refArray.Length)
        {
            fs.Close();
            HandleLargeFile(fileName);
            continue;
        }

        try
        {
            if ((_refArrayIndex + fs.Length) > _refArray.Length)
            {
                FlushBuffer();
                _refArrayIndex = 0;
                _entries.Clear();
            }
            long bytesToRead = fs.Length;
            int bytesRead = 0;
            int index = _refArrayIndex;
            while (bytesRead < bytesToRead)
            {
                bytesRead += fs.Read(_refArray, (index + bytesRead), (int)(bytesToRead - bytesRead));
            }
            CopyEntry entry = new CopyEntry
            {
                FileName = fileName.Replace(_sourceDir, _destDir),
                StartIndex = _refArrayIndex,
                Length = bytesRead
            };
            _entries.Add(entry);
            _refArrayIndex += bytesRead;
        }
        catch
        {
            _lstFailedFiles.Add(fileName);
        }
        finally
        {
            if (fs != null)
            {
                fs.Close();
            }
        }
    }
    foreach (string dir in Directory.GetDirectories(sourceDir))
    {
        string dirDest = dir.Replace(_sourceDir, _destDir);
        dirDest.CreateDirectory();
        ReadFiles(dir);
    }
}

With the using statement it can look like this :

private void ReadFiles(string sourceDir)
{
    foreach (string fileName in Directory.GetFiles(sourceDir))
    {
        using (FileStream fs = new FileStream(fileName, FileMode.Open))
        {
            if (fs.Length > _refArray.Length)
            {
                HandleLargeFile(fileName);
                continue;
            }
            try
            {
                //unchanged
            }
            catch
            {
                //unchanged
            }
        }
    }
    //unchanged
}

Also your void HandleLargeFile can be converted with the same technique to something better :

private void HandleLargeFile(string sourceFileName)
{
    //unchanged
    try
    {
        using (FileStream fsRead = new FileStream(sourceFileName, FileMode.Open))
        {
            int bytesToWrite = (int) fsRead.Length;
            using (FileStream fsWrite = bytesToWrite > INTERNAL_BUFFER
                    ? File.Create(destFileName, INTERNAL_BUFFER)
                    : File.Create(destFileName))
            {
                fsWrite.SetLength(bytesToWrite);
                //unchanged
            }
        }
    }
    catch(Exception ex)
    {

    }
}

You can apply the same technique to your FlushBuffer but I will leave that up to you as a small exercise.

Redundancy

Looking at this snippet :

int bufferSize = BUFFER_SIZE;
int writeIndex = 0;
byte[] buffer = new byte[BUFFER_SIZE];
int bytesRead = 0;

while (bytesToWrite > 0)
{
    bufferSize = Math.Min(bytesToWrite, BUFFER_SIZE);
    bytesRead = fsRead.Read(buffer, 0, bufferSize);
    fsWrite.Write(buffer, 0, bytesRead);
    bytesToWrite -= bytesRead;
    writeIndex += bytesRead;
}
  • int bufferSize = BUFFER_SIZE; BUFFER_SIZE is never used as a value + this variable can be declared in the while loop.

  • int writeIndex = 0; this variable is never used, you are just assigning some value to it but never using it.

  • int bytesRead = 0; this variable is also better to be declared inside your while loop as it's value is not used outside of it.

With those changes applied your code should look like this

byte[] buffer = new byte[BUFFER_SIZE];
while (bytesToWrite > 0)
{
    int bufferSize = Math.Min(bytesToWrite, BUFFER_SIZE);
    int bytesRead = fsRead.Read(buffer, 0, bufferSize);
    fsWrite.Write(buffer, 0, bytesRead);
    bytesToWrite -= bytesRead;
}

List<string> _lstFailedFiles = new List<string>(); this list is only being populated but never actually used, you can remove it completely.

Code style

As I stated at the beginning of the post you shouldn't be doing this in the first place but there are other situations where concepts like Null-propagation and ternary operators can be really useful so I will use your code as example, but you should not apply that directly, you should replace those lines with a using statement.

Null-propagation

This :

if (fs != null)
{
    fs.Close();
}

Can be replaced with a single line : fs?.Close();

You should have a null check here :

string dir = Path.GetDirectoryName(entry.FileName);
if (!Directory.Exists(dir))
{
    Directory.CreateDirectory(dir); // <--- HERE
}

Ternary operator

You can shorten this with the help of the ternary operator (I've already used that in one of the examples of the using statement):

if (entry.Length > INTERNAL_BUFFER)
{
    fs = File.Create(entry.FileName, INTERNAL_BUFFER);
}
else
{
    fs = File.Create(entry.FileName);
}
fs = entry.Length > INTERNAL_BUFFER
    ? File.Create(entry.FileName, INTERNAL_BUFFER)
    : File.Create(entry.FileName);

Empty try/catch block

Empty try/catch blocks are generally a bad practice, you either handle the exception in a way, or you fix the code so that there is no such exception.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.