3

Lets say we have the following model:

    public class Bar<T>:List<T>
    {
        public delegate void CollectionChangedDelegate();
        public event CollectionChangedDelegate CollectionChanged;
    }

    public class Foo
    {
        Bar<object> MyCollection = new Bar<object>();

        public Foo()
        {
            MyCollection.CollectionChanged += new Bar<object>.CollectionChangedDelegate(MyCollection_CollectionChanged);
        }

        public void MyCollection_CollectionChanged()
        {
            //Do Something
        }

        ~Foo() //Would this work ?
        {
            MyCollection.CollectionChanged -= MyCollection_CollectionChanged;
        }
    }

Could the destructor of Class Foo be called in this case ?

sm_
  • 2,572
  • 2
  • 17
  • 34
  • You should not use Destructors, use the IDisposable interface instead. Answer to your question: If no other living object holds an reference to the Foo class, they will be both collected, see: http://stackoverflow.com/questions/2775520/c-sharp-garbage-collector-cross-reference – Jeroen van Langen Aug 21 '13 at 11:23

2 Answers2

4

It is pointless to do so. Let's assume that there is such a subscription. Now, for us to get to the ~Foo method we must be being collected, so we must be unreachable. Because of how events work, subscriptions make us reachable (the publisher has a reference to the subscriber) - so we can deduce that MyCollection is also unreachable. If it wasn't, we wouldn't be being collected.

And if MyCollection is unreachable, then either it has already been collected, or it is about to be collected. There is no need to unsubscribe.

Remove ~Foo: it is pointless here; actually, it is worse than pointless - in addition to serving no useful purpose, it forces the garbage collector to push this object through an extra step (the finalizer queue).

You might, however, want to add some kind of deterministic clean-up that does this; as Joroen notes, IDisposable may be of use - however, it is hard to say whether it is appropriate in the case without knowing more context.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Okay, to make it simple .. Foo will be collected no matter what right ? And there is no need for the destructor ? And you said MyCollection has to be unreachable, how can i guarantuee that ? Lets say i made and instance of Foo, thus and instance of Bar (MyCollection) would be made too, how can i guarantuee that MyCollection will be collected and made unreachable by the end of the scope ? – sm_ Aug 21 '13 at 11:35
  • Also just for the Note class Foo represents a Business entity in the system i am working on, and is thoroughly used every where (Thats why IDisposable is not an option). I am trying to figure out if it could be the cause of a memory leak ? One of the developers in our team, claims it is and that he added the destructor and tested its effect on cleaning the memory. I would like to make sure of that – sm_ Aug 21 '13 at 11:41
  • @SirajMansour I was talking about the scenario where `~Foo` is being called: for that to happen, we can *deduce* that the collection was unreachable (otherwise we wouldn't have got there). As for keeping it unreachable - well, at the moment it is a private field - the only thing that can know about it is the `Foo` instance. Based on the *code shown* (and ***only*** the code shown), I say again: that `~Foo` is *worse* than useless. It has no good effects, and a definite negative effect. – Marc Gravell Aug 21 '13 at 12:12
  • One more question ? Could this pattern be a cause of a memory leak in case the destructor wasn't written ? In case it was ? Considering that Foo is a buisiness entity used everywhere in the system. – sm_ Aug 21 '13 at 13:05
  • 1
    @SirajMansour no, not in the code shown; again - in the scenario where this code runs, we **already know** that both the object and collection are unreachable. It serves no purpose ***whatsoever***. – Marc Gravell Aug 21 '13 at 13:19
3

first of all, I believe that you are trying to write ObservableCollection
You can find info here: http://msdn.microsoft.com/en-us/library/ms668604.aspx.

Now, if my object holds data-members that have a need to dispose, I would implement IDisposable and there I would dispose them, or at least remove the event subscription.
implment it like that: 

class Foo:Idisposable 
{
    public void Dispose(bool b)
    {
         MyCollection.CollectionChanged -= MyCollection_CollectionChanged;
    }
}
Amir Ofir
  • 126
  • 3
  • I'm not trying to write that, actually its already written, and i am working on a memory leak. I'am wondering if this would be a cause of a memory leak, because the class Foo represents a Business entity in the system i am working on, and is thoroughly used every where. – sm_ Aug 21 '13 at 11:39