File.Move() rename not working

My app takes "unclean" file names and "cleans" them up. "Unclean" file names contain characters like @, #, ~, +, %, etc. The "cleaning" process replaces those chars with "". However, I found that if there are two files in the same folder that, after a cleaning, will have the same name, my app does not rename either file. (I.e. ##test.txt and ~test.txt will both be named test.txt after the cleaning).

Therefore, I put in a loop that basically checks to see if the file name my app is trying to rename already exists in the folder. However, I tried running this and it would not rename all the files. Am I doing something wrong?

Here's my code:

        public void FileCleanup(List<string> paths)
    {
        string regPattern = (@"[~#&!%+{}]+");
        string replacement = "";

        Regex regExPattern = new Regex(regPattern);
        List<string> existingNames = new List<string>();
        StreamWriter errors = new StreamWriter(@"C:\Documents and Settings\joe.schmoe\Desktop\SharePointTesting\Errors.txt");
        StreamWriter resultsofRename = new StreamWriter(@"C:\Documents and Settings\joe.schmoe\Desktop\SharePointTesting\Results of File Rename.txt");

        开发者_如何转开发var filesCount = new Dictionary<string, int>();
        string replaceSpecialCharsWith = "_";

        foreach (string files2 in paths)

            try
            {
                string filenameOnly = Path.GetFileName(files2);
                string pathOnly = Path.GetDirectoryName(files2);
                string sanitizedFileName = regExPattern.Replace(filenameOnly, replacement);
                string sanitized = Path.Combine(pathOnly, sanitizedFileName);
                if (!System.IO.File.Exists(sanitized))
                {
                    System.IO.File.Move(files2, sanitized);
                    resultsofRename.Write("Path: " + pathOnly + " / " + "Old File Name: " + filenameOnly + "New File Name: " + sanitized + "\r\n" + "\r\n");   
                }

                else 
                {
                    existingNames.Add(sanitized);
                    foreach (string names in existingNames)
                    {

                        string sanitizedPath = regExPattern.Replace(names, replaceSpecialCharsWith);
                        if (filesCount.ContainsKey(sanitizedPath))
                        {
                            filesCount[names]++;
                        }
                        else
                        {
                            filesCount.Add(sanitizedPath, 1);
                        }

                        string newFileName = String.Format("{0},{1}, {2}", Path.GetFileNameWithoutExtension(sanitizedPath),
                        filesCount[sanitizedPath] != 0
                        ? filesCount[sanitizedPath].ToString()
                        : "",
                        Path.GetExtension(sanitizedPath));

                        string newFilePath = Path.Combine(Path.GetDirectoryName(sanitizedPath), newFileName);
                        System.IO.File.Move(names, newFileName);

                    }
                 }
              }
            catch (Exception e)
            {
                //write to streamwriter

            }

    }

}

Anybody have ANY idea why my code won't rename duplicate files uniquely?

---

• You do foreach (string names in existingNames), but existingNames is empty.
• You have your if (System.IO.File.Exists(sanitized)) backwards: it makes up a new name if the file doesn't exist, instead of when it exists.
• You make a string newFileName, but still use sanitizedPath instead of newFileName to do the renaming.
• The second parameter to filesCount.Add(sanitizedPath, 0) should be 1 or 2. After all, you have then encountered your second file with the same name.
• If filesCount[sanitizedPath] equals 0, you don't change the filename at all, so you overwrite the existing file.

---

In addition to the problem pointed out by Sjoerd, it appears that you are checking to see if the file exists and if it does exist you move it. Your if statement should be

     if (!System.IO.File.Exists(sanitized))
     {
         ...
     }    
     else 
     {
         foreach (string names in existingNames)
             {
                 ...    
             }
         }
     }

Update:

I agree that you should split the code up into smaller methods. It will help you identify which pieces are working and which aren't. That being said, I would get rid of the existingNames list. It is not needed because you have the filesCount Dictionary. Your else clause would then look something like this:

     if (filesCount.ContainsKey(sanitized))
     {
         filesCount[sanitized]++;
     }
     else
     {
       filesCount.Add(sanitized, 1);
     }

     string newFileName = String.Format("{0}{1}.{2}", 
                                        Path.GetFileNameWithoutExtension(sanitized),
                                        filesCount[sanitized].ToString(),
                                        Path.GetExtension(sanitized));

     string newFilePath = Path.Combine(Path.GetDirectoryName(sanitized), newFileName);
     System.IO.File.Move(files2, newFileName);

Please note that I changed your String.Format method call. You had some commas and spaces in there that looked incorrect for building a path, although I could be missing something in your implementation. Also, in the Move I changed the first argument from "names" to "files2".

---

A good way to make the code less messy would be to split it to methods as logical blocks.

FindUniqueName(string filePath, string fileName);
The method would prefix the fileName with a character, until the fileName is unique withing the filePath.

MoveFile(string filePath, string from, string to);
The method would use the FindUniqueName method if the file already exists.

It would be way easier to test the cleanup that way.

Also you should check if a file actually requires renaming:

if (String.Compare(sanitizedFileName, filenameOnly, true) != 0)
  MoveFile(pathOnly, fileNameOnly, sanitizedFileName);

private string FindUniqueName(string fileDirectory, string from, string to)
{
    string fileName = to;

    // There most likely won't be that many files with the same name to reach max filename length.
    while (File.Exists(Path.Combine(fileDirectory, fileName)))
    {
        fileName = "_" + fileName;
    }

    return fileName;
}

private void MoveFile(string fileDirectory, string from, string to)
{
    to = FindUniqueName(fileDirectory, from, to);

    File.Move(Path.Combine(fileDirectory, from), Path.Combine(fileDirectory, to));
}