0

What is an alternate/better way I can safely dispose of the variables purge and factory in the following code?

        public void Run( string XmlFragment ) {
            XmlNode xmlNode = null;

            try
            {
                xmlNode = Common.ConstructXmlNodeFromString(XmlFragment, "Params");
                var list = DataList();

                foreach (var item in list)
                {
                    var factory = new PurgerFactory(item);
                    IPurger purge = factory.Purger;

                    purge.Purge();

                    purge = null;
                    factory = null;
                }

                Common.PurgeEmail(SuccessEmail());
            }
            catch (Exception ex)
            {
                string errorMessage = $"Purge Error:  {ex.Message}. {Environment.NewLine} Stack Trace: {ex.StackTrace}";
                Common.PurgeEmail(FailEmail(errorMessage));
            }
        }
M. Azam
  • 21
  • 4
  • 4
    Meaning what, exactly? Do the `PurgerFactory` and `IPurger` types implement `IDisposable`? If so, why not just use `using` in the usual way? If not, what do you even mean by "dispose"? Your question is unclear. Please fix it so it includes a good [mcve], and an explanation of what it is that code does, what you want it to do instead, and what _specifically_ you are having trouble figuring out. – Peter Duniho Aug 07 '19 at 19:19
  • 1
    Do the types of variables `purge` or `factory` implement `IDisposable`? – Joe Sewell Aug 07 '19 at 19:19
  • 2
    This sounds like a question I had when switching from C++ to C#. C# uses a memory manager (garbage collector) that will clean up your objects when they are no longer being used so you don't need to do much of anything. – bwing Aug 07 '19 at 19:27

2 Answers2

2

As I think you know, C# has a garbage collector. So for normal objects that don't access an unmanaged resource, just letting the garbage collector clean them is fine.

If you want to deterministically close a managed resource the primary paradigm is inheriting from IDisposable and a using statement. This will call the Dispose function upon exiting the code block of the using statement.

If you otherwise want to clean stuff up, but you don't care when it happens you can use ~(MyType). This is called when the GC does a GC cycle, this is called the Finalizer. I personally haven't encountered a use-case for this vs IDisposable. But if you just want to make sure say a file, or otherwise is deleted when this email object is Garbage Collected, then it might be a good use case for you.

Jlalonde
  • 483
  • 4
  • 13
  • 2
    To add a word of warning: finalizers are very difficult to get right, they offer [few guarantees](https://ericlippert.com/2015/05/18/when-everything-you-know-is-wrong-part-one/), and most developers will not need to implement them. – Joe Sewell Aug 07 '19 at 19:42
  • Sadly in my answer I couldn't compose the absolute disaster of a code-base I've accidentally built in some side-projects playing with finalizers. – Jlalonde Aug 07 '19 at 19:43
0

Your code doesn't indicate whether purge or factory implement IDisposable. If they do, you would call

purge.Dispose();
factory.Dispose();

or use using which ensures that Dispose is called even if the method throws an exception:

using(var factory = new PurgerFactory(item))
{
    // do stuff
} // factory is disposed.

What we don't need to do is set variables to null. An object gets garbage collected sometime after there are no longer any references to it. It's true that setting factory = null removes that reference to the object, but the same thing happens anyway when the variable goes out of scope. So it's redundant.

There's really no need to try to force that to happen sooner. It's not like the object will get garbage collected the instant there are no more references to it, so we don't worry so much about that.

In this case, the variables are both declared within the foreach loop, so the references go out of scope immediately after each iteration of the loop. In other words, they already go out of scope right at the point when you're setting them to null.

That's part of what's awesome about .NET. Imagine if we had to set every reference variable to null. We'd need try/catch everywhere to make sure it happened. It would be as if every single object was disposable. What a nightmare. Instead it's handled for us in most normal scenarios so that we don't have to think about it or clutter our code with it.

Scott Hannen
  • 27,588
  • 3
  • 45
  • 62