2
\$\begingroup\$

I wrote a simple program that converts graphic files in a folder to a single PDF using Magic.NET. It works fine. I'm interested in general comments about the code as well as suggestions how I can make it run faster. Answers with some code are especially welcome.

static void Main(string[] args)
{
    Stopwatch stopWatch = new Stopwatch();
    stopWatch.Start();

    // Any() needs using System.Linq
    if ( Directory.EnumerateFileSystemEntries(args[0], "*.tif").Any() == false )
    {
        Console.WriteLine("No TIF files found.");
        Environment.Exit(-1);
    }

    string[] directory = Directory.GetFiles(args[0], "*.tif");

    // gets a directory name even when a relative path is given
    // http://stackoverflow.com/a/674495/2657875
    string filename = new FileInfo(directory[0]).Directory.FullName + ".pdf";

    // returns the pathname where the currently executing assembly resides
    MagickNET.Initialize(Path.GetDirectoryName
        (Assembly.GetExecutingAssembly().Location) + @"\MagicSettings");
    Console.WriteLine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location));
    using (MagickImageCollection collection = new MagickImageCollection())
    {
        foreach (string file in directory)
        {
            MagickImage image = new MagickImage(file);
            image.Quality = 100;
            image.Sharpen(0, 3);
            image.CompressionMethod = CompressionMethod.Group4;
            collection.Add(image);
        }

        try
        {
            collection.Write(filename);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }

        // Get the elapsed time as a TimeSpan value.
        TimeSpan ts = stopWatch.Elapsed;
        // Format and display the TimeSpan value.
        string elapsedTime = String.Format("{0:00}:{1:00}:{2:00}",
            ts.Hours, ts.Minutes, ts.Seconds);
        Console.WriteLine("Done in " + elapsedTime);
    }
}
\$\endgroup\$
1
\$\begingroup\$

I've got a couple quick notes. Overall it looks like this method will work.

Validating args[0]

The only validation that you do on args[0] is to check if there are any images in that directory, but you don't check other possible failure conditions, such as if the directory provided doesn't exist. For a user, no images being found and the directory itself not being found would have different actions required to fix the problem. It would be helpful to perform better validation here and providing meaningful feedback, and perhaps a different return code.

Duplicate Calls to Get Path

There are a couple instances that call Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location). This could be factored out to a variable.

Use TimeSpan Formatting

TimeSpan provides functionality for converting to a string in a particular format (see https://msdn.microsoft.com/en-us/library/ee372286(v=vs.110).aspx) In this case, I think you'd want the "g" format.

Return Codes

You return -1 in the event of no files being found, but you don't handle returning a different value if collection.Write(filename) fails. If this was being called from a script, I'd expect any failure to return a failed return code.

Catching Exceptions

While the exceptions thrown by collection.Write(filename) are likely all IO related, it would be helpful to catch at least some of the individual errors and displaying helpful information where possible. For example, if an exception was thrown because of a write permission issue, it would be helpful to let the user know that they need to correct their permissions, as opposed to hoping that the library you use provides a helpful description.

There are also some instances that I bet could throw an exception that you don't handle. For example, I would expect collection.Add(image) to throw an exception in some circumstances. If some exceptions are handles and displayed in a user-friendly way, I'd expect all of them to.

Commenting

Some of the comments are helpful, such as the reference to documentation for a method from StackOverflow. However, there are also some trivial comments (stopWatch.Elapsed is pretty clearly going to get the elapsed time). But there are also instances where comments would be helpful, such as what the values you set on the MagickImage are (what does image.Sharpen(0, 3) do?)

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.