Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should I implement IDisposable on classes handling external event to release event handlers? [duplicate]

I have a class that handles events from a WinForms control. Based on what the user is doing, I am deferencing one instance of the class and creating a new one to handle the same event. I need to unsubscribe the old instance from the event first - easy enough. I'd like to do this in a non-proprietary manner if possible, and it seems like this is a job for IDisposable. However, most documentation recommends IDisposable only when using unmanaged resources, which does not apply here.

If I implement IDisposable and unsubscribe from the event in Dispose(), am I perverting its intention? Should I instead provide an Unsubscribe() function and call that?


Edit: Here's some dummy code that kind of shows what I'm doing (using IDisposable). My actual implementation is related to some proprietary data binding (long story).

class EventListener : IDisposable
{
    private TextBox m_textBox;

    public EventListener(TextBox textBox)
    {
        m_textBox = textBox;
        textBox.TextChanged += new EventHandler(textBox_TextChanged);
    }

    void textBox_TextChanged(object sender, EventArgs e)
    {
        // do something
    }

    public void Dispose()
    {
        m_textBox.TextChanged -= new EventHandler(textBox_TextChanged);
    }
}

class MyClass
{
    EventListener m_eventListener = null;
    TextBox m_textBox = new TextBox();

    void SetEventListener()
    {
        if (m_eventListener != null) m_eventListener.Dispose();
        m_eventListener = new EventListener(m_textBox);
    }
}

In the actual code, the "EventListener" class is more involved, and each instance is uniquely significant. I use these in a collection, and create/destroy them as the user clicks around.


Conclusion

I'm accepting gbjbaanb's answer, at least for now. I feel that the benefit of using a familiar interface outweighs any possible downside of using it where no unmanaged code is involved (how would a user of this object even know that?).

If anyone disagrees - please post/comment/edit. If a better argument can be made against IDisposable, then I'll change the accepted answer.

like image 848
Jon B Avatar asked Nov 25 '22 22:11

Jon B


1 Answers

Yes, go for it. Although some people think IDisposable is implemented only for unmanaged resources, this is not the case - unmanaged resources just happens to be the biggest win, and most obvious reason to implement it. I think its acquired this idea because people couldn't think of any other reason to use it. Its not like a finaliser which is a performance problem and not easy for the GC to handle well.

Put any tidy-up code in your dispose method. It'll be clearer, cleaner and significantly more likely to prevent memory leaks and a damn sight easier to use correctly than trying to remember to un-do your references.

The intention of IDisposable is to make your code work better without you having to do lots of manual work. Use its power in your favour and get over some artificial "design intention" nonsense.

I remember it was difficult enough to persuade Microsoft of the usefulness of deterministic finalisation back when .NET first came out - we won the battle and persuaded them to add it (even if it was only a design pattern at the time), use it!

like image 66
gbjbaanb Avatar answered Jan 10 '23 19:01

gbjbaanb