HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

File Archiving Console App

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
consolearchivingfileapp

Problem

Overview

I wanted to create a small console app that would point at a given directory, evaluate the last accessed date of each file within that directory, and if the last access date was below a certain threshold, archive that file. The archive folder structure should match that of the source directory. I also decided to output the source folder structure to a log file, with an "ARCHIVED" flag against all those files that were archived.

Request

If you wouldn't mind reviewing my code for inefficiencies, mistakes, duplications, and anything else you think could be improved upon.

Note

I have included a decompress function which is not utilised in the script, but I needed for testing purposes.

Code

```
using System;
using System.Collections.Generic;
using System.IO;
using System.IO.Compression;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Archiving
{
class Program
{
static void Main(string[] args)
{
string sourcePath = @"e:\source\";
string destPath = @"e:\archive";
string logPath = @"e:\log files\";
string currentDate = DateTime.Now.ToShortDateString().Replace("/", "");

using (StreamWriter streamWriter = File.CreateText(logPath + currentDate + ".txt"))
{
try
{
string[] directories = Directory.GetDirectories(sourcePath, "*", SearchOption.AllDirectories);
foreach (string directory in directories)
{
streamWriter.WriteLine(directory);
string[] dirFiles = Directory.GetFiles(directory, ".", SearchOption.TopDirectoryOnly);
foreach (string file in dirFiles)
{
FileInfo fileInfo = new FileInfo(file);
if (FileExpiry(fileInfo))
{
string trimmedD

Solution


  • It's better to use App.config file to store such app settings as source path and destination path. It might have sense to provide these parameters via application args too. Hardcoding these settings is not the best option because you might run the application on system without e drive.



  • I suggest you use some logging library for logging in your application. I personally prefer NLog. So logPath setting and streamWriter will go away from your code.



  • Your code currently have too many levels of indentation. That result in too big method with big context which should be kept in mind.



  • There is no need to catch DirectoryNotFoundException and IOException separately, because you are handling them in exactly same way as global Exception. Even more - you can use global UnhandledException handler to catch and log all unhandled exceptions.



  • You can use EnumerateDirectories and EnumerateFiles methods to avoid grabbing all entries into array.



  • Of course you can move all file-arhiavation logic to some separate class (e.g. Arhiver) and call that class from simple and clean Main method.



  • Try not to use magic numbers in your code. E.g. it's not clear that expiration time is 5 days. It's better to create some constant variable with expressive name for that number or even provide this value from args/config.



  • Writing if return true else return false is totally same as return .



  • If you want to get all files from given directory you don't need to specify search pattern and search option.



After these improvements your console application code will be simple and clear - get application settings, set error handler and start archivation of expired files:

class Program
{
    private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

    static void Main(string[] args)
    {
        var sourcePath = ConfigurationManager.AppSettings["sourcePath"];
        var destinationPath = ConfigurationManager.AppSettings["destinationPath"];
        AppDomain.CurrentDomain.UnhandledException +=
           (o, e) => Logger.Error("An error occured", e);

        var archiver = new Archiver();
        archiver.ArchiveExpiredFiles(sourcePath, destinationPath);
    }
}


Archiver is also pretty straight-forward - it enumerates source directories and archives each expired file:

public class Archiver
{
    private TimeSpan expirationInterval = TimeSpan.FromDays(5);
    private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

    public void ArchiveExpiredFiles(string sourcePath, string destPath)
    {
        // you can check arguments here
        var expirationDate = DateTime.Now - expirationInterval;
        var sourceDirectory = new DirectoryInfo(sourcePath);
        var destDirectory = new DirectoryInfo(destPath);

        foreach (var sourceSubDirectory in sourceDirectory.EnumerateDirectories("*", SearchOption.AllDirectories))
        {
            var relativePath = sourceSubDirectory.GetPathRelativeTo(sourceDirectory);
            var destSubDirectory = new DirectoryInfo(Path.Combine(destPath, relativePath));

            ArchiveExpiredFiles(sourceSubDirectory, destSubDirectory, expirationDate);
        }
    }

    private void ArchiveExpiredFiles(DirectoryInfo sourceDirectory, DirectoryInfo destDirectory, DateTime expirationDate)
    {
        Logger.Info($"Processing directory {sourceDirectory.FullName}");

        foreach (var file in sourceDirectory.EnumerateFiles())
        {
            if (!file.IsExpired(expirationDate))
            {
                Logger.Info($"\t{file.FullName}");
                continue;
            }

            MoveFileToArchive(file, destDirectory);
            Logger.Info($"\t{file.FullName} - Archived");
        }
    }

    private void MoveFileToArchive(FileInfo file, DirectoryInfo destDirectory)
    {
        if (!destDirectory.Exists)
            destDirectory.Create();

        var compressedFile = file.IsCompressed() ? file : file.Compress();

        var destFilePath = Path.Combine(destDirectory.FullName, compressedFile.Name);
        if (File.Exists(destFilePath))
            destFilePath = Path.Combine(destDirectory.FullName, DateTime.Today.ToString("yyyyMMdd"), compressedFile.Name);

        compressedFile.MoveTo(destFilePath);
        file.Delete();
    }
}


Last missing part is extensions which I used in archiver - compressing files, checking wheter file was compressed or if its expired will look much cleaner

```
public static class FileSystemInfoExtensions
{
public static bool IsExpired(this FileInfo file, DateTime expirationDate)
{
return file.LastAccessTime < expirationDate;
}

public static FileInfo Compress(this FileInfo file)
{
// file-compression
throw new NotImplementedException();
}

public static bool IsCompressed(this FileInfo file)
{
return file.Extension == ".gz";
}

public static bool IsHidden(this FileInfo f

Code Snippets

class Program
{
    private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

    static void Main(string[] args)
    {
        var sourcePath = ConfigurationManager.AppSettings["sourcePath"];
        var destinationPath = ConfigurationManager.AppSettings["destinationPath"];
        AppDomain.CurrentDomain.UnhandledException +=
           (o, e) => Logger.Error("An error occured", e);

        var archiver = new Archiver();
        archiver.ArchiveExpiredFiles(sourcePath, destinationPath);
    }
}
public class Archiver
{
    private TimeSpan expirationInterval = TimeSpan.FromDays(5);
    private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

    public void ArchiveExpiredFiles(string sourcePath, string destPath)
    {
        // you can check arguments here
        var expirationDate = DateTime.Now - expirationInterval;
        var sourceDirectory = new DirectoryInfo(sourcePath);
        var destDirectory = new DirectoryInfo(destPath);

        foreach (var sourceSubDirectory in sourceDirectory.EnumerateDirectories("*", SearchOption.AllDirectories))
        {
            var relativePath = sourceSubDirectory.GetPathRelativeTo(sourceDirectory);
            var destSubDirectory = new DirectoryInfo(Path.Combine(destPath, relativePath));

            ArchiveExpiredFiles(sourceSubDirectory, destSubDirectory, expirationDate);
        }
    }

    private void ArchiveExpiredFiles(DirectoryInfo sourceDirectory, DirectoryInfo destDirectory, DateTime expirationDate)
    {
        Logger.Info($"Processing directory {sourceDirectory.FullName}");

        foreach (var file in sourceDirectory.EnumerateFiles())
        {
            if (!file.IsExpired(expirationDate))
            {
                Logger.Info($"\t{file.FullName}");
                continue;
            }

            MoveFileToArchive(file, destDirectory);
            Logger.Info($"\t{file.FullName} - Archived");
        }
    }

    private void MoveFileToArchive(FileInfo file, DirectoryInfo destDirectory)
    {
        if (!destDirectory.Exists)
            destDirectory.Create();

        var compressedFile = file.IsCompressed() ? file : file.Compress();

        var destFilePath = Path.Combine(destDirectory.FullName, compressedFile.Name);
        if (File.Exists(destFilePath))
            destFilePath = Path.Combine(destDirectory.FullName, DateTime.Today.ToString("yyyyMMdd"), compressedFile.Name);

        compressedFile.MoveTo(destFilePath);
        file.Delete();
    }
}
public static class FileSystemInfoExtensions
{
    public static bool IsExpired(this FileInfo file, DateTime expirationDate)
    {
        return file.LastAccessTime < expirationDate;
    }

    public static FileInfo Compress(this FileInfo file)
    {
        // file-compression
        throw new NotImplementedException();
    }

    public static bool IsCompressed(this FileInfo file)
    {
        return file.Extension == ".gz";
    }

    public static bool IsHidden(this FileInfo file)
    {
        return (file.Attributes & FileAttributes.Hidden) == FileAttributes.Hidden;
    }

    public static string GetPathRelativeTo(this DirectoryInfo subDirectory,
        DirectoryInfo directory)
    {
        if (!subDirectory.FullName.StartsWith(directory.FullName))
            throw new ArgumentException(
               $"{directory.FullName} is not parent of {subDirectory.FullName}");

        return subDirectory.FullName.Substring(directory.FullName.Length + 1);
    }
}

Context

StackExchange Code Review Q#148724, answer score: 5

Revisions (0)

No revisions yet.