patterncsharpMinor
Simple file recurser
Viewed 0 times
filesimplerecurser
Problem
I'm using C# for a project and thought I would get a feel for the language by making some simple programs. My current program is a simple file recurser that prints the names of files and directories. Any suggestions would be greatly appreciated.
```
using System;
using System.IO;
namespace FileRecurser
{
class Program
{
private static int indent = -2; // Start off at -2 so first indent
// starts at 0.
static void Main(string[] args)
{
if (args.Length != 1)
{
Console.WriteLine("Usage: recurser ");
return;
}
// Check if directory exists.
string directory = args[0];
if (!Directory.Exists(directory))
{
Console.WriteLine("Error: {0} doesn't exist!", directory);
return;
}
Console.WriteLine("Printing files for {0}\n", directory);
PrintDirectoryFiles(directory);
}
private static void PrintDirectoryFiles(string directory)
{
indent += 2;
// If any subdirectories.
string[] subdirectories = Directory.GetDirectories(directory);
if (subdirectories.Length != 0)
{
foreach (var subdirectory in subdirectories)
{
// Change color to reflect directory.
Console.ForegroundColor = ConsoleColor.Cyan;
Console.WriteLine(
"{0}{1}:",
new String(' ', indent),
Path.GetFileName(subdirectory)
);
Console.ResetColor();
PrintDirectoryFiles(subdirectory);
Console.WriteLine();
}
}
// Print file names.
string[] files = Directory.GetFiles(directory);
if (
```
using System;
using System.IO;
namespace FileRecurser
{
class Program
{
private static int indent = -2; // Start off at -2 so first indent
// starts at 0.
static void Main(string[] args)
{
if (args.Length != 1)
{
Console.WriteLine("Usage: recurser ");
return;
}
// Check if directory exists.
string directory = args[0];
if (!Directory.Exists(directory))
{
Console.WriteLine("Error: {0} doesn't exist!", directory);
return;
}
Console.WriteLine("Printing files for {0}\n", directory);
PrintDirectoryFiles(directory);
}
private static void PrintDirectoryFiles(string directory)
{
indent += 2;
// If any subdirectories.
string[] subdirectories = Directory.GetDirectories(directory);
if (subdirectories.Length != 0)
{
foreach (var subdirectory in subdirectories)
{
// Change color to reflect directory.
Console.ForegroundColor = ConsoleColor.Cyan;
Console.WriteLine(
"{0}{1}:",
new String(' ', indent),
Path.GetFileName(subdirectory)
);
Console.ResetColor();
PrintDirectoryFiles(subdirectory);
Console.WriteLine();
}
}
// Print file names.
string[] files = Directory.GetFiles(directory);
if (
Solution
I don't like how you use static field to indicate level of indentation, I think it would be better if you passed it down the recursion in parameters of the methods.
And while you're at it, you should probably separate your concerns: don't have everything in your
Also, there is no need to check whether a collection is empty before iterating it, it just unnecessarily clutters the code.
And you should think about handling exceptions: there might be directories you don't have access to. And there's also a chance that a directory was deleted while you were iterating its siblings, so you should handle that too.
And while you're at it, you should probably separate your concerns: don't have everything in your
Program class, but have one class that recurses directories, and one that writes them out. That way, you can reuse them if you want to write the output in a different way (say in a GUI app) or if you want to recurse through something else than files (say, registry entries).Also, there is no need to check whether a collection is empty before iterating it, it just unnecessarily clutters the code.
And you should think about handling exceptions: there might be directories you don't have access to. And there's also a chance that a directory was deleted while you were iterating its siblings, so you should handle that too.
Context
StackExchange Code Review Q#11270, answer score: 2
Revisions (0)
No revisions yet.