On 2021/01/22 23:58, Marco van de Voort via lazarus wrote:
Op 2021-01-22 om 22:56 schreef Michael Van Canneyt via lazarus:
I have some time this weekend, I will commit it.
Is it really a good idea to accept msec=1000 for TryEncodeTimeInterval
in a general unit like Dateutils?
Maybe - can you construe a situation where the allowance would cause
undue error or miscalculation of a datetime? It's essentially an
allowance for rounding. If your date-time calculation tries to add
999.999ms or 0.9999 sec to a datetime value, would you really be harmed
if one full second was added? If so, what does it say for the occasion
where you add 0.1116s and either 0.112s or 0.111s are physically added
after rounding/truncation - is the prior any worse? Currently rounding
works in all cases (000.000 up to 999.99444) except the very final 0.5ms
of the range, and code has to be added to everything to fix that small
remainig range because rounding perfectly *valid* time values to ms =
1000 will error out.
I can and have demonstrated a case where NOT allowing msec=1000 is in
fact detrimental and breaks in current production code (as far as the
MySQL connection DATETIME values go) for any datetime fraction nearing
lim{ dt --> 1000ms }. Allow me another demonstration without the DB stuff:
Reproducible test case:
Getting the current ms can be easily written as:
ms := Round(Frac(Now() * 86400)*1000); // ms is the fraction of the
seconds multiplied by 1000 and rounded.
Here is a full test-case anyone can run:
procedure timetest;
var ms : Integer;
dtms : TDateTime;
begin
try
repeat
ms := Round(Frac(Now() * 86400)*1000);
dtms := EncodeTime(00,00,00,ms);
until false;
finally
WriteLn('ms: ',ms);
end;
end;
It errors out rather quick with the 1000ms culprit problem.
Now as a programmer i have to either TRUNCATE the time, which is not
great because 12.9ms will become 12ms in stead of 13ms, or add special
code to test for ms=1000, and what is worse, that code is almost always:
if (ms=1000) then dtms := EncodeTime(00,00,01,00) else dtms :=
EncodeTime(00,00,00,ms);
I hope it is not lost on any readers how silly that is - I mean what
ELSE could one possibly end up doing/meaning in the case of 1000ms?.
I will even argue that can be detrimental since I have seen fixes like this:
if (ms=1000) then ms:=999;
dtms := EncodeTime(00,00,00,ms);
which drives me to tears of course, but one could almost understand why
the programmer did that, sacrificing accuracy to get rif of a silly
exception. It isn't FPC's fault though, that is just a bad programmer.
As programmers our duty is to make our code conform strictly to the
rules of the underlying systems, and I have been quite a vocal advocate
for this on other fora - but rules should have clear reasons, not simply
be rules for the sake of having rules - which comes to my challenge of
showing a case where it is detrimental.
There was a proposal to compute the specific datetime value for the
MySQL connector itself in Floating point space, which is a perfect
solution (and I will be very happy with that too), but does it address
the full spectrum of plausible cases? Would this need to be fixed in the
PostGres (etc.) connectors too? Fixing it at the root of date-time
calculations when the change is indeed sensible, AND barring
demonstrable "bad" cases (I hasten to add), seems the appropriate thing
to do.
Lastly...
In other datetime calcs, if you try to add a day that doesn't exist (ex:
dt := EncodeDate(2021,02,30); ) then the reason it is wrong is clear and
it should error out, that date does not exist in current calendars.
Adding 25ms to 02:15:04.975ms simply lands you at 02:15:05.000 - It's
hard to for me to find a reasonable example of where that will cause
undue error or a result that is fundamentally wrong.
The only thing I can think that might be a problem is a programmer
seeing that EncodeTime(02,15,40,1000) yields --> 02:15:41. It's not
technically wrong, but would this be unexpected? Would it confuse
people? (Moreso than throwing an exception?)
If this is the solution we go with, this 1000ms case should probably be
mentioned clearly in the documentation.
PS: I work with precision time a lot, GPS tracking systems etc, so for
my use cases this is probably way more of a niggle than everyone else,
so I concede my opinion doesn't carry all that much weight, but I really
do not see the harm in fixing this for everyone - unless it can be shown
to be detrimental, in which case I will happily accept it and appreciate
being shown it.
Thank you for caring!
Ryan
--
_______________________________________________
lazarus mailing list
[email protected]
https://lists.lazarus-ide.org/listinfo/lazarus
