Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was being troubled a lot by Robocopy (or maybe anti-virus or maybe Network Hardware). I copy files from the Dynamic View of Clearcase to the local machine for a fresh build. The copy would frequently fail due to:

2012/12/06 15:35:07 ERROR 64 (0x00000040)...

The specified network name is no longer available.

The server and all network hardware resides in another geographical location, so there is no way to ascertain the hardware issues. The anti-virus can never be disabled as per some policy. The Snapshot View cannot be used as per another weird policy or prejudice.

I am left with just one option: to make Robocopy resilient. The following is what I have come up with, which is a wrapper for Robocopy. Please review this.

Summary:

  1. Call Robocopy with parameters
  2. Give it some time and certain number of tries for proper execution
  3. First try is allowed 10 minutes. The subsequent tries will have an increment of 5 minutes. Maximum 5 tries and 30 minutes are allowed.
  4. Catch all Robocopy error codes(0, 1 and 2 are success codes) and re-try.

namespace ResilientRobocopy
    {
        class Program
        {
            static int Main(string[] args)
            {
                string commandLine = string.Empty;
                int count = 0;
                foreach (string str in args)
                {
                    string temp = string.Empty;
                    if ((count == 0) || (count == 1))
                    {
                        temp = "\"" + str + "\"" + " ";
                    }
                    else
                    {
                        temp = str + " ";
                    }
                    commandLine = commandLine + temp;
                    count += 1;
                }
                Console.WriteLine("--------------------------------------------------------------------");
                Console.WriteLine("Robocopy Command Line: " + commandLine);

                int returnCode = -1;
                int tries = 0;

                while (((returnCode == -1) || (returnCode == -2)) && (tries != 5))
                {
                    Console.WriteLine("Calling Robocopy");
                    returnCode = StartCopy(commandLine, tries);
                    tries += 1;
                }
                Console.WriteLine("--------------------------------------------------------------------");
                return returnCode;
            }

            static int StartCopy(string commandLine, int tries)
            {
                Process robocopy = new Process();
                try
                {
                    robocopy = Process.Start("C:\\Windows\\SysWOW64\\robocopy.exe", commandLine);
                    int timeLimit = 120 + (60 * tries);
                    for (int i = 0; i <= timeLimit; i++)
                    {
                        Thread.Sleep(1000 * 5);
                        if (!robocopy.HasExited)
                        {
                            continue;
                        }
                        else
                        {
                            if (robocopy.ExitCode > 2)
                            {
                                Console.WriteLine("Robocopy exited with code: " + robocopy.ExitCode.ToString());
                                Console.WriteLine("Retrying...");
                                return -1;
                            }
                            else
                            {
                                Console.WriteLine("Robocopy exited with code: " + robocopy.ExitCode.ToString());
                                Console.WriteLine("Robocopy Done!");
                                return robocopy.ExitCode;
                            }
                        }
                    }

                    if (!robocopy.HasExited)
                    {
                        Console.WriteLine("Killing Robocopy. Took too much time. Try try again till you succeed...");
                        robocopy.Kill();
                        return -1;
                    }
                    else
                    {
                        if (robocopy.ExitCode > 2)
                        {
                            Console.WriteLine("Robocopy exited with code: " + robocopy.ExitCode.ToString());
                            Console.WriteLine("Retrying...");
                            return -1;
                        }
                        else
                        {
                            Console.WriteLine("Robocopy exited with code: " + robocopy.ExitCode.ToString());
                            Console.WriteLine("Robocopy Done!");
                            return robocopy.ExitCode;
                        }
                    }
                }

                catch(Exception ex)
                {
                    Console.WriteLine("Exception: " + ex.ToString());
                    return -2;
                }
                finally
                {
                    robocopy.Close();
                }
            }
        }
    }
share|improve this question

1 Answer 1

Naming

The method StartCopy() implies that the copy process can also be stopped. So we should better rename it to CopyFiles().

Refactoring

First step would be to refactor the composition of the commandline to a separate method which we will name ComposeCommandline.

private static String ComposeCommandline(string[] args)
{
    int count = 0;
    StringBuilder sb = new StringBuilder(1024);

    foreach (string str in args)
    {
        if ((count == 0) || (count == 1))
        {
            sb.Append("\"").Append(str).Append("\"");
        }
        else
        {
            sb.Append(str);
        }
        sb.Append(" ");
        count += 1;
    } 
    return sb.ToString();
}

The next what sprung in our eyes is the multiple calling to Console.WriteLine, so we create a method Print(String,params Object[]) so this can be called with additional parameters also.

private static void Print(String message, params Object[] args)
{
    String content = String.Empty;
    if (args.Length != 0)
    {
        content = String.Format(message, args);
    }
    else
    {
        content = message;
    }
    Console.WriteLine(content);
}  

Now we should put some of the text which has been used to call Console.WriteLine and also the magic numbers 1000 * 5, 120 and 60 to constants.

private static const String Separator = "--------------------------------------------------------------------";
private static const String RobocopyExitMessage = "Robocopy exited with code: {0}";
private static const String RobocopyDone = "Robocopy Done!";
private static const String RobocopyRetry = "Retrying...";
private static const String RobocopyCommandline = "Robocopy Command Line: {0}";
private static const String RobocopyCalling = "Calling Robocopy";
private static const String RobocopyKilling = "Killing Robocopy. Took too much time. Try try again till you succeed...";
private static const String RobocopyException = "Exception: {0}";
private static const int MilliSecondsToSleep = 5000;  
private static const int Initial5SecondIterations = 120;
private static const int Additional5SecondIterations = 60;

Now it is also easier to change the strings, as this only happen in one place.

If we now extract the while loop to a overloaded method CopyFiles()

private int CopyFiles(String commandLine)
{
    int returnCode = -1;
    int tries = 0;
    int maxSleepingIterations = Initial5SecondIterations + (Additional5SecondIterations * tries);
    while (((returnCode == -1) || (returnCode == -2)) && (tries != 5))
    {
        Print(RobocopyCalling);
        maxSleepingIterations = Initial5SecondIterations + (Additional5SecondIterations * tries);
        returnCode = StartCopy(commandLine, maxSleepingIterations);
        tries += 1;
    }
    return returnCode;
}

the Main() method would look like

static int Main(string[] args)
{
    String commandLine = ComposeCommandline(args);

    Print(Separator);
    Print(RobocopyCommandline,commandLine);

    int returnCode = CopyFiles(commandLine);

    Print(Separator);

    return returnCode;
}  

Now we should refactor the initial StartCopy() method.

if (!robocopy.HasExited)
{
    continue;
}

if we invert the condition we will reduce the indentationlevel by 1 which is more readable. And if we refactor the former else part to just use break we reduce the code duplication a lot.

static int CopyFiles(string commandLine, int maxSleepingIterations)
{
    Process robocopy = new Process();
    try
    {
        robocopy = Process.Start("C:\\Windows\\SysWOW64\\robocopy.exe", commandLine);
        for (int i = 0; i <= maxSleepingIterations; i++)
        {
            Thread.Sleep(MilliSecondsToSleep);
            if (robocopy.HasExited)
            {
                break;
            }
        }
        if (robocopy.HasExited)
        {
            Print(RobocopyExitMessage, robocopy.ExitCode);
            if (robocopy.ExitCode > 2)
            {
                Print(RobocopyRetry);;
                return -1;
            }
            else
            {
                Print(RobocopyDone);
                return robocopy.ExitCode;
            }
        }
        else
        {
            Print(RobocopyKilling);
            robocopy.Kill();
            return -1;
        }
    }
    catch(Exception ex)
    {
        Print(RobocopyException, ex.ToString());
        return -2;
    }
    finally
    {
        robocopy.Close();
    }
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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