[c#] C# Timer or Thread.Sleep

I am running a windows service and using a loop and Thread.Sleep to repeat a task, would it be better to use a timer method?

If yes a code example would be great

I am currently using this code to repeat

int curMinute;
int lastMinute = DateTime.Now.AddMinutes(-1).Minute;

while (condition)
{
   curMinute = DateTime.Now.Minute;

   if (lastMinute < curMinute) {
         // do your once-per-minute code here
         lastMinute = curMinute;
   }

   Thread.Sleep(50000);      // sleeps for 50 seconds

   if (error condition that would break you out of this) {
       break;      // leaves looping structure
   }
}

You can use either one. But I think Sleep() is easy, clear and shorter to implement.

Yes, using a Timer will free up a Thread that is currently spending most of its time sleeping. A Timer will also more accurately fire every minute so you probably won't need to keep track of lastMinute anymore.

Not quite answering the question, but rather than having

if (error condition that would break you out of this) {
    break;  // leaves looping structure
}

You should probably have

while(condition && !error_condition)

Also, I'd go with a Timer.

I required a thread to fire once every minute (see question here) and I've now used a DispatchTimer based on the answers I received.

The answers provide some references which you might find useful.

Beware that calling Sleep() will freeze the service, so if the service is requested to stop, it won't react for the duration of the Sleep() call.

I agree as well, using a timer is the best option. I have tried a solution similar to yours in the past and started having issues where the loop would misfire, and I would have to wait for another Thread.Sleep() before it would fire again. Also, it did cause all sorts of issues with stopping the service, I would get constant errors about how it wasn't responding and had to be closed.

@Prashanth's code should be exactly what you need.

It's important to understand that your code will sleep for 50 seconds between ending one loop, and starting the next...

A timer will call your loop every 50 seconds, which isn't exactly the same.

They're both valid, but a timer is probably what you're looking for here.

A timer is a better idea, IMO. That way, if your service is asked to stop, it can respond to that very quickly, and just not call the timer tick handler again... if you're sleeping, the service manager will either have to wait 50 seconds or kill your thread, neither of which is terribly nice.

I have used both timers and Thread.Sleep(x), or either, depending on the situation.

If I have a short piece of code that needs to run repeadedly, I probably use a timer.

If I have a piece of code that might take longer to run than the delay timer (such as retrieving files from a remote server via FTP, where I don't control or know the network delay or file sizes / count), I will wait for a fixed period of time between cycles.

Both are valid, but as pointed out earlier they do different things. The timer runs your code every x milliseconds, even if the previous instance hasn't finished. The Thread.Sleep(x) waits for a period of time after completing each iteration, so the total delay per loop will always be longer (perhaps not by much) than the sleep period.

I would have to say a sleep is a better implementation with a state machine behind it. This would still keep you in control of the application at all times, but allowing any response needed at any specific time. This also will handle timer callbacks that are shorter than the "Processing execution time in the loop"

For example..

<!-- language: c# -->
public enum State
{
    Idle = 0,
    Processing = 1,
    Stop = 100,
}
public void Run()
{
    State state = State.Idle;   // could be a member variable, so a service could stop this too

    double intervalInSeconds = 60;
    System.DateTime nextExecution = System.DateTime.Now.AddSeconds(intervalInSeconds);
    while (state != State.Stop)
    {
        switch (state)
        {
            case State.Idle:
                {
                    if (nextExecution > System.DateTime.Now)
                    {
                        state = State.Processing;
                    }
                }
                break;
            case State.Processing:
                {
                    // do your once-per-minute code here

                    // if you want it to stop, just set it state to stop.
                    // if this was a service, you could stop execution by setting state to stop, also
                    // only time it would not stop is if it was waiting for the process to finish, which you can handle in other ways

                    state = State.Idle;
                    nextExecution = System.DateTime.Now.AddSeconds(intervalInSeconds);
                }
                break;
            default:
                break;
        }

        System.Threading.Thread.Sleep(1);
    }
}