File archiver, archive files into a zip file

Question

I've created a simple little file archiver that will run through a given directory and extract your files into an archive folder. It's pretty cool if I do so say myself, I would like some critique on what I've done, and some ideas on how I could better this program, better syntax, easier ways to do this, etc..

using System;
using System.IO;
using System.IO.Compression;

namespace ArchiveCreator
{
    class Program
    {
        //When program is run successfully 
        //this will be the output
        public string Success(string input)
        {
            Console.ForegroundColor = ConsoleColor.White;
            Console.WriteLine(input);
            return input;
        }

        //When program encounters an error 
        //this will be the output
        public string Warn(string input)
        {
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine(input);
            return input;
        }

        //When program has information to show
        //this will be the output
        public string Say(string input)
        {
            Console.ForegroundColor = ConsoleColor.DarkCyan;
            Console.WriteLine(input);
            return input;
        }

        //Main method
        static void Main(string[] args)
        {
            //These variables are used to create a
            //random string that will be used as the
            //zip files name
            var chars = "abcdefghijklmnopqrstuvwyxzABCDEFGHIJKLMNOPQRSTUVWXYZ";
            var stringChars = new char[8];
            var random = new Random();

            //Info is used as provide the type of
            //information that will be displayed
            //by the program
            Program info = new Program();

            //Create the zip file name
            for (int i = 0; i < stringChars.Length; i++)
            {
                stringChars[i] = chars[random.Next(chars.Length)];
            }
            string finalString = new String(stringChars);

            info.Say("Starting file extraction..");

            string userName = Environment.UserName;
            string startDir = $"c:/users/{userName}/test_folder";
            string zipDir = $"c:/users/{userName}/archive/{finalString}.zip";
            string dirName = $"c:/users/{userName}/archive";

            //Check if the directory exists
            if (Directory.Exists(dirName))
            {
                info.Say("Directory already exists, resuming");
            }
            else
            {
                //Create it if it doesn't
                Directory.CreateDirectory(dirName);
            }

            try
            {
                ZipFile.CreateFromDirectory(startDir, zipDir);
            }
            catch (Exception e)
            {
                info.Warn($"Error: {e}");
            }
            info.Success($"Extracted files successfully to: {zipDir}");
            info.Say("Press enter to exit..");
            Console.ReadLine();
        }
    }
}

janos · Accepted Answer · 2016-05-20 20:20:44Z

Code organization

The Main method should be very short, and have almost no logic at all, simply call other methods and create very few objects. Something like this:

static void Main(string[] args)
{
    string filename = archiveCreator.GenerateRandomFilename();

    string userName = Environment.UserName;
    string startDir = $"c:/users/{userName}/test_folder";
    string zipFile = $"c:/users/{userName}/archive/{finalString}.zip";
    string dirName = $"c:/users/{userName}/archive";

    Say("Starting file extraction..");

    var archiveCreator = new ArchiveCreator();
    if (archiveCreator.CreateDirIfNotExists(dirName)) {
        archiveCreator.CreateArchive(startDir, zipFile);
    }

    Say("Press enter to exit..");
    Console.ReadLine();
}

Printing messages

This and the similar methods simply print messages:

public string Success(string input)
{
    Console.ForegroundColor = ConsoleColor.White;
    Console.WriteLine(input);
    return input;
}

As such, these have nothing to do with the logic of creating archives, so these methods would be better in another class. Or, you could keep them within this class, but make them static.

Also, I don't see a point in return input, it would be better to make these methods void.

Error handling

Consider this piece:

try
{
    ZipFile.CreateFromDirectory(startDir, zipDir);
}
catch (Exception e)
{
    info.Warn($"Error: {e}");
}
info.Success($"Extracted files successfully to: {zipDir}");

If an exception occurs in the try block, the program prints a warning, but it also prints "Extracted files successfully", which is just not true. Probably you want to move that line inside the try block.

Also, the program doesn't check that the directory could be created. It should.

Naming

Symbols in a program should have good names, to make it easier for readers to understand the program and how the different elements work together. For example, stringChars, Program, info don't describe well what they are:

var stringChars = new char[8];

Program info = new Program();

I recommend renaming them, for example:

stringChars -> filename or randomFilename
Program -> ArchiveCreator
info -> archiveCreator

I like this but info is literally just information of what's going on, I think that's just fine for a name..? — 13aal
– 13aal, Commented May 20, 2016 at 23:15

Stack Exchange Network

File archiver, archive files into a zip file

1 Answer 1

Code organization

Printing messages

Error handling

Naming

You must log in to answer this question.

Hot Network Questions

File archiver, archive files into a zip file

1 Answer 1

Code organization

Printing messages

Error handling

Naming

You must log in to answer this question.

Related

Hot Network Questions