0

I've been working on a c# application a while ago and I had to use Timers. Now I am about to change this app from console one to windows servie. I try to optimize my code a bit and make sure it's gonna work okey as a service. So Timers...

This is pretty much how I have my timers declared now:

firstTimer = new System.Timers.Timer(5000); 
firstTimer.Elapsed += FirstEvent;
firstTimer.Enabled = false;

I can see here https://msdn.microsoft.com/en-us/library/zb0225y6%28v=vs.110%29.aspx that Timer class implements IDisposable so I can just call Dispose() and expect everything would be okey? My class has 3 timers in it. Should I just call Dispose() on every timer there is or implement IDisposable to my class and define Dispode method like this:

public void Dispose()
        {
            firstTimer.Elapsed -= FirstEvent;
            secondTimer.Elapsed -= SecondEvent;
            thirdTimer.Elapsed -= ThirdEvent;

            firstTimer.Dispose();
            secondTimer.Dispose();
            thirdTimer.Dispose();

            this.Dispose();
        }

Or is this bad approach? Do I even need to do this firstTimer.Elapsed -= FirstEvent; ? Or will timer get automaticly disposed after -= ?

Since I implemented IDisposable on my class I guess it's correct to use using block on it.

static void Main(string[] args)
{
    using (Myclass mc = new Myclass())
    {
        mc.init();
    }
}

init method is where I have my timers declarations and all related stuff.

To sum up my question. Is this approach correct? Can I add something? Maybe what I did was stupid? There's lots of other topics about disposing timers but I can't really get an answer out of them.

Thanks for the insight.

Dashovsky
  • 137
  • 9

1 Answers1

1

Your Dispose method should look like:

public void Dispose()
{
    firstTimer.Dispose();
    secondTimer.Dispose();
    thirdTimer.Dispose();
}

Recursively calling this.Dispose() is a very bad idea of course. Removing the event handlers is not necessary.


Or alternatively you could implement the more complex (and often not necessary) way of implementing the finalizer ~ClassName, Dispose and Dispose(bool) described in this article. But that is mainly if you use both managed and unmanaged resources.

Dirk
  • 10,668
  • 2
  • 35
  • 49
  • *Recursively calling this.Dispose()* is not a bad idea of course; it will certainly result in `StackOverFlowException`. I would not call it is bad idea; bad idea is something we say when it will work but it is not good. In this case, it won't even work at all. – Sriram Sakthivel Apr 17 '15 at 09:51