Losing argument value

Question

I am coding something right now, and I am passing a string to the constructor. The manner in which the string is being generated is not changing in any way, but it (when I run the debug tools in Visual Studio Community) loses the value the first time, but shows a value most other times. Intermittently, the value is reporting that the string is null, or the value it should be.

Now, I really don't know how to document exactly what it is that I am doing, so here's the basics.

The first part is the definition of TempDir. I am using these temporary directories as testing directories that automatically kill themselves, and delete the contents, when the TempDir (and the test) go out of scope.

FINAL, WORKING, NO-LOST VALUE VERSION

public class TempDir : IDisposable
{
    private readonly string _path;
    public string ActiveDirectory => _path.Substring(_path.LastIndexOf('/') + 1, (_path.Length - _path.LastIndexOf('/') - 1));

    public string Path
    {
        get
        {
            return _path;
        }
    }
    public TempDir(string path) : this(path, false) { }
    public TempDir(string path, bool KillExisting)
    {
        _path = path;
        if(!KillExisting)
            return;
        if(Directory.Exists(_path))
            Directory.Delete(_path);
    }
    public void Dispose( )
    {
        if(System.IO.Directory.Exists(_path))
            Directory.Delete(_path, true);
    }

    public static implicit operator String(TempDir dir) => dir._path;
}

Now, this is the code that I am sending to a constructor. The TempDir's ActiveDirectory is being sent to a constructor, where NameOfThing should be the result of the first argument, and the second argument is also a string. The first one is intermittently working, the second always works.

TempDir dir = new TempDir(Environment.GetFolderPath(Environment.SpecialFolders.LocalApplicationData) + "/First/Second/NameOfThing")

I am seriously so lost on this, and from what I can tell, I think a thread may be changing something on me without my knowing

EDIT:

I can now "reliably" get it to pass every time, but I have to slowly walk through every single line of code. Running it normally without debugging through every line fails every time, but debugging slowly makes it pass every time.

Code constructing TempDir:

protected static string PackagesLocation = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) + "/Sloth/Notes/";
protected static TempDir TestPackLoc = new TempDir(PackagesLocation + "NPackageTests");
protected static NPackage TestPack = new NPackage(TestPackLoc.ActiveDirectory);

Test Method creating the page

[TestMethod]
public void GeneratesLayoutAndResourcesDirectory( )
{
    string key = "GeneratesLayoutAndResourcesDictionary";
    TestPack.CreatePage(key);
    if(!Directory.Exists(TestPackLoc + "/" + key + "/res") && !Directory.Exists(TestPackLoc + "/" + key + "/layout.xml"))
        Assert.Fail( );
}

Okay, so the behavior of the lost value was, I think, because C# was calling the garbage collector inappropriately. @mason mentioned that for the TempDir type, instead of implementing a destructor, I should implement the IDisposable. Now, it works reliably, and consistently. I have no clue why implementing the destructor did this, but swapping it out for IDisposable works just fine.

Credit for solution goes to @mason

Answer 1

A destructor isn't really necessary here. The IDisposable pattern could be used instead. There's more to it than just implementing the interface, you also need to handle the object properly in any object that uses your IDisposable. You can implement a using statement.

using(var tempDir = new TempDir(arguments))
{
    //you can use tempDir inside here
} //tempDir's Dispose method is automatically called here
//since tempDir is out of scope here, the directory will have been deleted already

Anytime that an object implements IDisposable, you should wrap it in a using statement like above, or call its Dispose method in a finally block to make sure it gets removed properly. Here's what the try/catch/finally version would be like:

TempDir tempDir = null;

try
{
    tempDir = new TempDir(arguments);
    //now you can use tempDir here
}
catch(Exception ex)
{
    //log the exception. Optionally rethrow. Do not leave catch block empty
}
finally
{
    if(tempDir != null)
    {
        tempDir.Dispose();
    }
}

Most of the time I prefer the using block, because it makes the scope of the variable clearer.

You could also use a destructor to make sure that if someone forgets to call Dispose or wrap the object in a using block that the unmanaged resource (the directory) gets cleaned up properly.

public class TempDir : IDisposable
{
    private readonly string _path;

    public string ActiveDirectory => _path.Substring(_path.LastIndexOf('/') + 1, (_path.Length - _path.LastIndexOf('/') - 1));

    public string Path => _path;

    public TempDir(string path) : this(path, false) { }

    public TempDir(string path, bool KillExisting)
    {
        if(string.IsNullOrEmpty(path))
        {
            throw new ArgumentException($"{nameof(path)} cannot be null or empty.");
        }

        _path = path;

        if(KillExisting && Directory.Exists(_path))
        {
            Directory.Delete(_path);
        }

        //why not call Directory.CreateDirectory(_path) here?
    }

    public void Dispose( )
    {
        Cleanup();
    }

    ~TempDir()
    {
        Cleanup();
    }

    private void Cleanup()
    {
        if(Directory.Exists(_path))
        {
            Directory.Delete(_path, true);
        }
    }

    public static implicit operator String(TempDir dir) => dir._path;
}

There is no need to manually set objects to null as the garbage collector will handle de-allocating the memory for you.

Answer 2

You are not encapsulating your private variables properly. It could be that you use the public _path variable outside of your class and inadvertently setting it to something else.

An improved version of your TempDir class might look like this:

public class TempDir
{
    private string _path;

    public TempDir(string path)
        : this(path, false) { }

    public TempDir(string path, bool killExisting)
    {
        _path = path;

        if (!killExisting) return;

        if (Directory.Exists(_path))
            Directory.Delete(_path);
    }

    public string Path => _path;

    public string ActiveDirectory
        => _path.Substring(_path.LastIndexOf('/') + 1, 
           _path.Length - _path.LastIndexOf('/') - 1);

    ~TempDir()
    {
        if (System.IO.Directory.Exists(_path))
            Directory.Delete(_path, true);
        _path = null;
    }

    public static implicit operator string(TempDir dir)
    {
        return dir._path;
    }
}