Temporary unzip - is this a valid use of the IDisposable interface?

Question

I want to encapsulate the process of unzipping a zip file, making the files available for use, and then automatically cleaning them up when they are no longer needed. I did this with a class that implements the IDisposable interface, so that I can instantiate it with "using" and the files will be cleaned up when going out of scope, eliminating the need to specifically delete the files. The class, TempUnzip, can therefore be used thus:

    static void AccessZipFileContents(string zipFilePath)
    {
        using (var temp = new TempUnzip(zipFilePath)
        {
            var tempPath = temp.TempPath;
            if (tempPath != null)
            {
                // read/use the files in tempPath
            }
        } // files automatically get deleted when it goes out of scope! Woohoo!
    }

Here is the implementation of the TempUnzip class:

using System.IO;
using System.IO.Compression;
public class TempUnzip : IDisposable
{
    public TempUnzip(string zipFilePath)
    {
        try
        {
            var tempFolderName = Path.GetRandomFileName();
            var tempFolder = Path.GetTempPath();
            var tempPath = Path.Combine(tempFolder, tempFolderName);
            Directory.CreateDirectory(tempPath);
            ZipFile.ExtractToDirectory(zipFilePath, tempPath);
            TempPath = tempPath;
        }
        catch (Exception) { TempPath = null; }
    }

    public readonly string TempPath;

    public void Dispose()
    {
        try
        {
            if (TempPath != null)
                Directory.Delete(TempPath);
        }
        catch (Exception) { }
    }
}

Is this a valid use of IDisposable?

• If so, do I need to implement the full standard IDisposable pattern?

• If not, is there a better way to encapsulate the creation and destruction of files in such a way that they're tied to the lifetime of an object, or should I avoid this altogether?

Answer 1

Is this a valid use of IDisposable?

From the documentation:

Provides a mechanism for releasing unmanaged resources.

Files on local disk are certainly unmanaged resources. Thus, this use fits with the stated purpose of IDisposable.

If so, do I need to implement the full standard IDisposable pattern?

You can. The usual cautions about finalizers need to be considered, but you already linked to those. It certainly won't hurt.

If not, is there a better way to encapsulate the creation and destruction of files in such a way that they're tied to the lifetime of an object, or should I avoid this altogether?

I also like a functional approach for this kind of problem. That would make your example look something like this:

static void AccessZipFileContents(string zipFilePath)
{
    ZipManager.RunWithZipExtracted(zipFilePath, (string tempPath) =>
    {
        if (tempPath != null)
        {
            // read/use the files in tempPath
        }
    } // files automatically get deleted when it goes out of scope! Woohoo!
}

//from ZipManager.cs...
static void RunWithZipExtracted(string zipLocation, Action<string> toRun)
{
    var tempPath = CalculateTempPath();
    try
    {
        ExtractZip(zipLocation, tempPath);
        toRun(tempPath);
    }
    finally
    {
        DeleteFolder(tempPath);
    }
 } //private methods left as exercise for the reader

A pattern like that avoids the problem of "what if they don't call Dispose?" entirely.

Answer 2

This is a situation where having a finalizer is probably a good idea, but the MS pattern is predicated on the idea of public objects having finalizers, which is almost always a bad idea. Instead, resources that will require finalization-based cleanup should be encapsulated in privately-held objects whose references are never exposed to the outside world. Because the inner objects are private, there's no need for them to use the IDisposable pattern--instead they may be designed in whatever fashion best fits the requirement.

Since trying to close a file handle more than once may have catastrophic consequences, and it's possible (though rare) for a finalizer to execute while other code is using an object (or even performing Dispose upon it!), writing a robust class can be difficult. A nasty issue that arises is that file accesses can block but finalizer actions shouldn't. One could work around that by creating a thread whose purpose was to wait until the file is supposed to be closed and deleted, and which would then close and delete the file. Even if the attempt to delete the file gets blocked, other finalizer actions could continue apace. Unfortunately, eagerly creating a thread for the purpose of allowing safe finalizer-based cleanup is apt to be a waste of resources, but thread creation seems like an excessively-heavyweight task to be performing within a finalizer. I don't know what the best solution is.

Answer 3

In this instance I would say that it is a good example of IDisposable, if Dispose isn't called straight away that you've finished using it then it's not the end of the world and quickly calling this for the same zip file wouldn't cause an exception as you're using unique temp folders each time; which again you're not leaving file pointers left open on the files, etc. The only issue I could see is that if disk space was really tight you might want to expose the delete folder as a method to allow the option of that being called directly and then Dispose is just used to clean up after for all cases.

Note: in this example you probably want to call Directory.Delete(TempPath, true); because the method you're calling will throw an IOException if the folder isn't empty (something your catch will hide) - see https://msdn.microsoft.com/en-us/library/62t64db3%28v=vs.110%29.aspx