-2

I have an app that allows my users to turn on and off a timer to track their time spent on a certain task. The timer runs a clock used to show the elapsed time to the user, much like a stopwatch.

The code below has worked as I thought it should for a few years now. However, when the app is run on Win 10, sometimes the "time" rate speeds up by 2 or 3 times during a session. If the user restarts the app, it may run at normal speed.

Win 10 Delphi 10.3

procedure TfmTimeCard.btnTimerClick(Sender: TObject);
begin
  if btnTimer.Caption = 'Start &Timer' then
  begin
    btnTimer.Down := True;
    btnTimer.Caption := 'Stop &Timer';
    pnlTimer.Color := clPurple;
    btnResume.Enabled := True;
    btnAssign.Enabled := False;
    Timer1.Enabled := true;
    UpdateTimer.Enabled := True;
    ElapsedTime := ElapsedTime;
    //btnPostRecord.Enabled := False;
    btnCancel.Enabled := False;
    btnDeleteTimeCard.Enabled := False;
  end
  else
  begin
    btnTimer.Down := False;
    btnTimer.Caption := 'Start &Timer';
    pnlTimer.ParentColor := True;
    btnResume.Enabled := False;
    btnAssign.Enabled := True;
    pnlTimer.Color := clMoneyGreen;
  end;
end;

procedure TfmTimeCard.Timer1Timer(Sender: TObject);
begin
  if btnTimer.Caption = 'Stop &Timer' then
  begin
    ElapsedTime := ElapsedTime + 0.0000115740;
    cxClock1.time := ElapsedTime;
    cxTimeEditTimer.Time := ElapsedTime;
  end;
end;
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Disable CPU SpeedStep in BIOS? – gabr Mar 01 '19 at 20:39
  • 3
    Instead of `ElapsedTime := ElapsedTime + 0.0000115740`, why don't you add the *actual* elapsed time? – MartynA Mar 01 '19 at 21:07
  • 3
    Delphi has a [stopwatch](http://docwiki.embarcadero.com/Libraries/Tokyo/en/System.Diagnostics.TStopwatch). Why use a timer when you want a stopwatch? Timer events are not guaranteed to be "on time". – J... Mar 01 '19 at 21:17
  • Related : [On timers...](https://stackoverflow.com/a/45881019/327083) Adding a constant interval in a timer like this is ridiculously fragile. I'm amazed it ever worked reliably. I doubt that it did - I suspect people just didn't notice the discrepancies. – J... Mar 01 '19 at 21:20
  • 3
    I have a feeling some employees got over-paid as a result of this code... A `TTimer` doesn't fire exactly at every interval. It fires after a *delay* of a given time. You should instead be keeping track of exact timestamps and comparing the difference between them. `DateUtils` is stuffed with functions to compare various time ranges. For example `MinutesBetween()` lets you get total number of minutes between start and end time. – Jerry Dodge Mar 01 '19 at 21:27
  • 2
    Also, storing business logic state in the text of a button should be avoided like the plague. Visual controls are for displaying state and receiving input from the user. They are not data stores and should not be used as such. – J... Mar 01 '19 at 21:35
  • 2
    What a strange way to count time! Even if the timer was the most accurate thing in the world (which is far from it), your timing would still slip because a second is actually 0.0000115740740740740... of a day. – Sertac Akyuz Mar 01 '19 at 21:53
  • To add, suppose your app is doing some big heavy loop, and goes into the "(Not Responding)" state. During this time, Windows Messages are blocked and therefore the timer doesn't fire. And don't expect it to fire for each second it was blocked either. All a `TTimer` is is just a way to put some sort of automated delay between executions. There's no guarantee of actual time. – Jerry Dodge Mar 02 '19 at 01:50

1 Answers1

10

This is a terrible way to keep track of elapsed time with a TTimer. TTimer is not a real-time timer, or even an accurate timer. It is based on the WM_TIMER window message, which is

a low-priority message. The GetMessage and PeekMessage functions post this message only when no other higher-priority messages are in the thread's message queue.

Don't calculate your ElapsedTime based on how often the TTimer fires its OnTimer event. Keep track of the current time when starting the TTimer, and then subtract that value from the next current time whenever the OnTimer event is eventually generated. That will give you a more real elapsed time.

Try something more like this:

uses
  ..., System.DateUtils;

private
  StartTime: TDateTime;
  ElapsedSecs: Int64;

procedure TfmTimeCard.btnTimerClick(Sender: TObject);
begin
  if btnTimer.Tag = 0 then
  begin
    btnTimer.Tag := 1;
    ...
    ElapsedSecs := 0;
    StartTime := Now;
    Timer1.Enabled := true;
    ...
  end
  else
  begin
    btnTimer.Tag := 0;
    ...
    ElapsedSecs := SecondsBetween(Now, StartTime);
    Timer1.Enabled := false;
    ...
  end;
end;

procedure TfmTimeCard.Timer1Timer(Sender: TObject);
begin
  if btnTimer.Tag = 1 then
  begin
    ElapsedSecs := SecondsBetween(Now, StartTime);
    // use ElapsedSecs as needed ...
  end;
end;

Or:

uses
  ..., Winapi.Windows;

private
  StartTime: DWORD;
  ElapsedSecs: Integer;

procedure TfmTimeCard.btnTimerClick(Sender: TObject);
begin
  if btnTimer.Tag = 0 then
  begin
    btnTimer.Tag := 1;
    ...
    ElapsedSecs := 0;
    StartTime := GetTickCount;
    Timer1.Enabled := true;
    ...
  end
  else
  begin
    btnTimer.Tag := 0;
    ...
    ElapsedSecs := (GetTickCount - StartTime) div 1000;
    Timer1.Enabled := false;
    ...
  end;
end;

procedure TfmTimeCard.Timer1Timer(Sender: TObject);
begin
  if btnTimer.Tag = 1 then
  begin
    ElapsedSecs := (GetTickCount - StartTime) div 1000;
    // use ElapsedSecs as needed ...
  end;
end;

Or:

uses
  ..., System.Diagnostics;

private
  SW: TStopwatch;
  ElapsedSecs: Integer;

procedure TfmTimeCard.btnTimerClick(Sender: TObject);
begin
  if not SW.IsRunning then
  begin
    ...
    ElapsedSecs := 0;
    SW := TStopWatch.Start;
    Timer1.Enabled := true;
    ...
  end
  else
  begin
    ...
    SW.Stop;
    ElapsedSecs := Trunc(SW.Elapsed.TotalSeconds);
    Timer1.Enabled := false;
    ...
  end;
end;

procedure TfmTimeCard.Timer1Timer(Sender: TObject);
begin
  if SW.IsRunning then
  begin
    ElapsedSecs := Trunc(SW.Elapsed.TotalSeconds);
    // use ElapsedSecs as needed ...
  end;
end;
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • I think the suggestion to use `GetTickCount` should probably come with the usual warning about wraparound. Really, there are enough tools in the toolbox these days that I'd probably suggest it's something for niche applications where you really want it for a specific reason - for most applications a `TStopwatch` or any other timekeeping method should probably be preferred. – J... Mar 02 '19 at 10:22
  • Also don't like using the button's `.Tag` property to store business state. A properly named boolean field is a better solution, I think - if for no other reason than code readability. OP should probably wrap all of this business logic into a class, to be honest, but it doesn't hurt to at least keep to some level of standard even if leaving it coupled to this form. – J... Mar 02 '19 at 10:27