7

Is it bad practice to use such while loop? Maybe it is better to use Stopwatch, or maybe this solution has some pitfalls?

    public void DoWork()
    {
        //do some preparation
        DateTime startTime = DateTime.Now;
        int rowsCount = 0;
        int finalCount = getFinalCount();
        do
        {
            Thread.Sleep(1000);
            rowsCount = getRowsCount(); // gets rows count from database, rows are added by external app.
        } while (rowsCount < finalCount && DateTime.Now - startTime < TimeSpan.FromMinutes(10));

    }

I saw this article Implement C# Generic Timeout, but it is too complex to use in simple scenarios - you need to think about sync of threads, is it proper to abort them or not and so on.

Community
  • 1
  • 1
Andriy Kizym
  • 1,756
  • 2
  • 14
  • 29
  • 3
    What do you want to do with Creating a TimeOut exactly? If you tell your whole scenerio, maybe we can offer different way to use. – Serkan Hekimoglu Feb 16 '11 at 16:16
  • Related / duplicate: http://stackoverflow.com/questions/3195030/thread-timeout-in-c – RQDQ Feb 16 '11 at 16:16
  • Just need to stop work after some period of time - start work, if it runs more than 10 minutes stop the work. The thread that calls the method can be blocked. – Andriy Kizym Feb 16 '11 at 16:18
  • 1
    @anderhil: what is that work? Do you control that code? Do you realize that by sleeping, you're not working? You may want to provide a better example. – R. Martinho Fernandes Feb 16 '11 at 16:21
  • @Martinho Fernandes: updated the question. getRowsCount get rows from database table. Table is updated by external app, I need to wait for specific amount of rows in database which i get with getFinalCount method. But there can be a situation that nothing is updated and to prevent the infinite while loop i want to wait some time and then stop the work. – Andriy Kizym Feb 16 '11 at 16:32
  • Much better now :) From here it seems like your solution is reasonable. I can suggest one improvement: like you said, and Jim suggests below, use a Stopwatch. Depending on how the table-filling task behaves, you may also want to tune the amount spent sleeping. It may be worth checking more or less often. – R. Martinho Fernandes Feb 16 '11 at 16:42
  • @Martinho Fernandes: yes, this code is for example, I'm using the setting of sleeping and timeout time in app.config. – Andriy Kizym Feb 16 '11 at 16:56

2 Answers2

16

As I understand it, you want your method to do some work until it's done or until some period of time has elapsed? I would use a Stopwatch for that, and check the elapsed time in a loop:

void DoWork()
{
    // we'll stop after 10 minutes
    TimeSpan maxDuration = TimeSpan.FromMinutes(10);
    Stopwatch sw = Stopwatch.StartNew();
    DoneWithWork = false;

    while (sw.Elapsed < maxDuration && !DoneWithWork)
    {
        // do some work
        // if all the work is completed, set DoneWithWork to True
    }

    // Either we finished the work or we ran out of time.
}
Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
  • Thanks for the answer. I thought about Stopwatch at first, then I decided that DateTime is simpler, can you explain why Stopwatch is better? I thought it's implementation based on DateTime... – Andriy Kizym Feb 16 '11 at 16:59
  • 1
    @anderhil: I think Stopwatch is much more readable :). The only other advantage over DateTime.Now is that Stopwatch *can* have a higher resolution (not that it matters for you problem, though). – R. Martinho Fernandes Feb 16 '11 at 17:05
  • 8
    I wouldn't use `DateTime` because the date can change underneath me come Daylight Saving Time. It could cause the code to wait too long (an hour and 10 minutes) or it could cause the code not to wait long enough. But `Stopwatch` knows how many minutes have elapsed, regardless of what time it is. – Jim Mischel Feb 16 '11 at 18:19
  • This is a nice clean solution - thanks. I like the use of stopwatch because `sw.Elapsed < maxDuration` is much clearer to read than formulae involving `DateTime.Now()`. – Richard Moore Jan 21 '19 at 13:37
1

It is better to use the System.Timers.Timer class.

John Pick
  • 5,562
  • 31
  • 31