public static class DirectoryInfoExtensions
{
public static void CopyTo(this String Source, String Destination, Boolean Overwrite)
{
// Hold directory information
var SourceDirectory = new DirectoryInfo(Source);
var DestinationDirectory = new DirectoryInfo(Destination);
// Throw an error is the source directory does not exist
if (SourceDirectory.Exists == false) {
throw new DirectoryNotFoundException();
}
// Create the destination directory
if (DestinationDirectory.Exists == false) {
DestinationDirectory.Create();
}
// Loop through the files and copy them
var SubFiles = SourceDirectory.GetFiles();
for (int i = 0; i < SubFiles.Length; i++) {
var NewFile = Path.Combine(
DestinationDirectory.FullName,
SubFiles[i].Name
);
SubFiles[i].CopyTo(NewFile, Overwrite);
}
// Loop through the directories and call this function
var SubDirectories = SourceDirectory.GetDirectories();
for (int i = 0; i < SubDirectories.Length; i++) {
var NewDirectory = Path.Combine(
DestinationDirectory.FullName,
SubDirectories[i].Name
);
SubDirectories[i].CopyTo(NewDirectory, Overwrite);
}
}
}
var SourceDirectory = new DirectoryInfo(@"C:\Users\");
SourceDirectory.CopyTo(@"C:\Users_Backup\", true);
Refactorings
No refactoring yet !
Rik Hemsley
November 11, 2008, November 11, 2008 20:04, permalink
1. This should not be an extension of class String. A string should not know anything about copying files. I would make this a static method of a class named e.g. FileUtils.
2. Your indentation is messed up.
3. You should not be explicitly naming CLR classes. Use the C# aliases.
4. Variable identifiers should begin with a lower case letter.
5. You loop through collections using an indexer but the indexer is unused save for the iteration. You should iterate using foreach instead. Also note that .NET will soon provide an IEnumerable<> interface for iterating through directory entries, which is more sensible.
6. You name variables 'NewDirectory' and 'NewFile' when in fact they are not directories, files or even structures which may be treated as such. They are paths to directories and files. If you are going to use type inference, you should name your identifiers correctly, lest you incur the wrath of your fellow developers.
7. There is no documentation on the method, but its exact behaviour is impossible to guess
without reading the code. The user cannot know what will happen if the destination directory exists and they do not specify the 'overwrite' flag.
8. The 'overwrite' flag should not be of boolean type. An enumeration would make it much more clear to the reader of client code what the intent is. For example:
Directory.Copy(sourceDirectoryPath, destinationDirectoryPath, ExistingDirectoryPolicy.Overwrite);
9. The comments are redundant - the code is perfectly readable (apart from the issues with naming, indentation and iteration) as it stands.
10. You create many variables which are referenced only once after initialization. Have you considered inlining any of them? It think in this case most could be inlined to make the code even easier to read.
11. Your DirectoryNotFoundException does not specify the path that was not found.
12. You can end up blowing the stack if the filesystem contains a link (e.g. symlink, junction point) from one point in the tree to higher up and you end up trapped in a loop.
There are probably many more issues which I haven't covered. I'm writing some code with similar goals myself at the moment so I'm interested to see what you come up with.
rikkus
November 11, 2008, November 11, 2008 20:17, permalink
Here's a quick sketch of some improvements I'd make. This probably has lots of problems remaining, for example, I didn't deal with links either!
Cheers,
Rik
namespace Whatever
{
public static class FileUtils
{
public void CopyDirectory
(
string sourcePath,
string destinationPath,
ExistingDestinationDirectoryPolicy existingDestinationDirectoryPolicy
)
{
if (!Directory.Exists(sourcePath))
{
throw new DirectoryNotFoundException(string.Format("Source directory not found: {0}", sourcePath));
}
if (Directory.Exists(destinationPath))
{
bool cancel = false;
ApplyExistingDestinationDirectoryPolicy(destinationPath, existingDestinationDirectoryPolicy, ref cancel);
if (cancel)
{
return;
}
}
else
{
Directory.CreateDirectory(destinationPath);
}
CopyDirectories(sourcePath, existingDestinationDirectoryPolicy);
CopyFiles(sourcePath, destinationPath);
}
private void CopyDirectories(string sourceDirectoryPath, ExistingDestinationDirectoryPolicy existingDestinationDirectoryPolicy)
{
foreach (var directoryPath in Directory.GetDirectories(sourceDirectoryPath))
{
CopyDirectory
(
directoryPath,
Path.Combine(sourceDirectoryPath, Path.GetFileName(directoryPath)),
existingDestinationDirectoryPolicy
);
}
}
private static void CopyFiles(string sourceDirectoryPath, string destinationDirectoryPath)
{
foreach (var filePath in Directory.GetFiles(sourceDirectoryPath))
{
File.Copy(filePath, Path.Combine(Path.GetFileName(filePath), destinationDirectoryPath));
}
}
private static void ApplyExistingDestinationDirectoryPolicy
(
string destinationPath,
ExistingDestinationDirectoryPolicy existingDestinationDirectoryPolicy,
ref bool cancel
)
{
switch (existingDestinationDirectoryPolicy)
{
case ExistingDestinationDirectoryPolicy.DeleteExisting:
Directory.Delete(destinationPath, true /* recursive */);
break;
case ExistingDestinationDirectoryPolicy.CancelOnExisting:
cancel = true;
break;
case ExistingDestinationDirectoryPolicy.ThrowOnExisting:
throw new DirectoryExistsException(destinationPath);
default:
// Carry on!
break;
}
}
}
}
Moonshield
November 11, 2008, November 11, 2008 23:34, permalink
Just add the parameters validations you need and you'll be good I think
namespace ConsoleApplication1
{
class Program
{
static void Main(string[] args)
{
FileUtils.CopyDirectory(@"C:\MyFiles\Test", @"C:\MyFiles\Test2", true);
Console.WriteLine("Already done...");
Console.ReadLine();
}
public static class FileUtils
{
public static void CopyDirectory(string pSource, string pDestination, bool pOverrideExistingFile)
{
(from p in Directory.GetFiles(pSource, "*.*", SearchOption.AllDirectories)
group p by Path.GetDirectoryName(p) into GroupByDirectory
select new
{
Directory = GroupByDirectory.Key.Replace(pSource, pDestination),
Files = (from p in GroupByDirectory
select new
{
SourceFile = p,
Destination = p.Replace(pSource, pDestination)
}).ToList()
})
.ToList()
.ForEach(item =>
{
if (!Directory.Exists(item.Directory))
Directory.CreateDirectory(item.Directory);
item.Files.ForEach(fileInfo => File.Copy(fileInfo.SourceFile, fileInfo.Destination, pOverrideExistingFile));
});
}
}
}
}
Because the .NET Framework does not contain a function which allows you to copy a directory from one location to another, I thought I would write one:
http://gatekiller.co.uk/Post/DirectoryInfo.CopyTo
Please let me know what you think. Any refactorings to make the code more elegant would be much appreciated.
Cheers
Stephen