View Issue Details

IDProjectCategoryView StatusLast Update
0031233FPCRTLpublic2017-02-11 18:05
ReporterBart BroersmaAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindowsOS VersionWin7
Product VersionProduct Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0031233: Incorrect result of YearsBetween()
DescriptionYearsBetween(1956-12-01,2015-12-01) incorrectly returns 58, it shoud be 59.
Steps To Reproduceprogram yearsdiff;

uses
  SysUtils, DateUtils;
var
  D1, D2: TDateTime;
begin
  D1 := (EncodeDate(1956, 12, 1));
  D2 := (EncodeDate(2015, 12, 1));
  writeln('FPC: ',YearsBetween(D1, D2));
end.

It ouputs (win32 fpc 3.0.2RC1 and trunk):
FPC: 58
Additional InformationSee related forum discussion: http://forum.lazarus.freepascal.org/index.php/topic,35432.0.html

For dates > 31-12-1899 a precise value can be returned that does not rely on ApproxDaysPerYear.

While one could debate that, since both dates have the same day and month, and therefore the last year counting "is not full", when it compares 1958-12-01 with 2016-12-01, it returns 58 as expected.

Also adding 1 hour to 2015-12-01 does not change the outcome.
TagsNo tags attached.
Fixed in Revision35423
FPCOldBugId
FPCTarget
Attached Files
  • yearsbetween.diff (913 bytes)
    Index: packages/rtl-objpas/src/inc/dateutil.inc
    ===================================================================
    --- packages/rtl-objpas/src/inc/dateutil.inc	(revision 35221)
    +++ packages/rtl-objpas/src/inc/dateutil.inc	(working copy)
    @@ -1282,11 +1282,19 @@
     
     
     Function YearsBetween(const ANow, AThen: TDateTime): Integer;
    +var
    +  yy, mm, dd: Word;
     begin
    -  Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerYear);
    +  if (ANow >= -DateDelta) and (AThen >= -DateDelta) and
    +     (ANow <= MaxDateTime) and (AThen <= MaxDateTime) then
    +  begin
    +    PeriodBetween(ANow, AThen, yy , mm, dd);
    +    Result := yy;
    +  end
    +  else
    +    Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+TDateTimeEpsilon)/ApproxDaysPerYear);
     end;
     
    -
     Function MonthsBetween(const ANow, AThen: TDateTime): Integer;
     begin
       Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerMonth);
    
    yearsbetween.diff (913 bytes)
  • yearsbetween-commented.diff (1,152 bytes)
    Index: packages/rtl-objpas/src/inc/dateutil.inc
    ===================================================================
    --- packages/rtl-objpas/src/inc/dateutil.inc	(revision 35221)
    +++ packages/rtl-objpas/src/inc/dateutil.inc	(working copy)
    @@ -1280,13 +1280,25 @@
         Result:=Result+0.5;
     end;
     
    -
    +{
    +  For years between 0001/01/01 and 9999/12/31 the function is exact,
    +  for other years the funcion may be off by 1 if mm-dd in both dates
    +  are the same. This is due to the use of ApproxDaysPerYear.
    +}
     Function YearsBetween(const ANow, AThen: TDateTime): Integer;
    +var
    +  yy, mm, dd: Word;
     begin
    -  Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerYear);
    +  if (ANow >= -DateDelta) and (AThen >= -DateDelta) and
    +     (ANow <= MaxDateTime) and (AThen <= MaxDateTime) then
    +  begin
    +    PeriodBetween(ANow, AThen, yy , mm, dd);
    +    Result := yy;
    +  end
    +  else
    +    Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+TDateTimeEpsilon)/ApproxDaysPerYear);
     end;
     
    -
     Function MonthsBetween(const ANow, AThen: TDateTime): Integer;
     begin
       Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerMonth);
    

Activities

Bart Broersma

2017-01-15 19:41

reporter   ~0097504

Last edited: 2017-01-15 19:42

View 2 revisions

IMO YearsBetween(SomeDate, SomeDate exactly 1 year later) should return 1.
I'm not sure what Delphi does though.
Also should Time be part of the equation?

Bart Broersma

2017-01-15 19:59

reporter   ~0097505

Fpc vs Delphi 7 results:

1-12-1956 -> 1-12-2015
FPC: 58
Delphi: 58

1-12-1958 -> 1-12-2016
FPC: 58
Delphi: 58

1/1/2016 -> 1/1/2017
FPC: 1
Delphi: 1

Bart Broersma

2017-01-15 20:09

reporter   ~0097506

1-12-1956 -> 1-12-2015 17:59:00
FPC: 58
Delphi 58

1-12-1956 -> 1-12-2015 18:00:00
FPC: 59
Delphi 59

So we need 3/4 extra day here.

wp

2017-01-15 22:40

reporter   ~0097509

Last edited: 2017-01-15 22:41

View 2 revisions

There's a problem with leap years: Suppose a period enclosing Feb 29, say Jan 1 2016 --> Dec 31 2016. This is not a full year, because the interval is 365 days, but a full leap year is 366 days. Using your correction of 0.75 days the calculation would yield 365.75 / 365.25 > 1 --> 1 year which is wrong.

In my opinion, the only correct way is to look at years, months and days individually. This is what PeriodBetween does:

function YearsBetween(ANow, AThen: TDateTime): Integer;
var
  y,m,d: Word;
begin
  PeriodBetween(ANow, AThen, y, m, d);
  Result := y;
end;

In the same way, the function MonthsBetween could also be correctect (it will certainly have the same rounding issues due to the average month length):

function MonthsBetween(ANow, AThen: TDateTime): Integer;
var
  y, m, d: Word;
begin
  PeriodBetween(ANow, AThen, y, m, d);
  Result := y*12 + m;
end;

Bart Broersma

2017-01-19 18:21

reporter   ~0097599

Last edited: 2017-01-19 18:43

View 2 revisions

> Using your correction of 0.75 days
??
I did not suggest to add a correction.
I merely tried to point out that the input requires 3/4 day extra (on second date) to give the correct answer.

Also, suggested fix is only correct for years >= 0001-01-01, since DecodeDate will return all zero's for dates before 1 AD.

As a remark: PeriodBetween may not be "correct" if the interval spans the date the Gregorian calendar was introduced (IIRC then some number of days were skipped).
And the meaning of a "leap year" may also be undetermined before that time...
(was the year 0004 AD a leap year?)

Bart Broersma

2017-01-19 18:42

reporter   ~0097600

Last edited: 2017-01-19 18:42

View 2 revisions

This would solve the "years before Christ" problem:

function YearsBetween(ANow, AThen: TDateTime): Integer;
var
  yy, mm, dd: Word;
begin
  if (ANow >= -DateDelta) and (AThen >= -DateDelta) and
     (ANow <= MaxDateTime) and (AThen <= MaxDateTime) then
  begin
    PeriodBetween(ANow, AThen, yy , mm, dd);
    Result := yy;
  end
  else
    Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+TDateTimeEpsilon)/ApproxDaysPerYear);
end;

The comparison with MaxDateTime is needed because DecodeDate truncates any date greater than MaxDateTime to MaxDateTime.

Bart Broersma

2017-01-21 15:01

reporter   ~0097618

If acceptable, I can create a patch based upon solution in note 0097600.
I also think some comments on the behaviour of YeasrBetween might be in order (especially that it can be off-by-one due to rounding errors for dates before 1 AD and dates > MaxDateTime).

wp

2017-01-21 17:43

reporter   ~0097622

> I also think some comments on the behaviour of YeasrBetween might be in order
> (especially that it can be off-by-one due to rounding errors for dates before 1 > AD and dates > MaxDateTime)

Really? These are very long time periods, and the average year length here really is very close to 365.25 as it is assumed in ApproxDaysPerYear. The problem, I think, is in the short periods, and these are covered by the PeriodBetween in the first condition in your function.

Bart Broersma

2017-01-22 00:57

reporter   ~0097625

The problem will still be there for say "jan 1st 2 BC" -> "jan 1st 1 BC", since it uses the aloritm that introduces this rounding errors.

wp

2017-01-22 09:59

reporter   ~0097628

Me stupid: Although the dates since the reference date are large, differences can be small...

Bart Broersma

2017-01-22 12:22

reporter   ~0097631

;-)

Michael Van Canneyt

2017-01-29 11:48

administrator   ~0097775

@Bart: if you could create a patch, as you suggest, that would be hugely appreciated.

Bart Broersma

2017-01-30 19:01

reporter  

yearsbetween.diff (913 bytes)
Index: packages/rtl-objpas/src/inc/dateutil.inc
===================================================================
--- packages/rtl-objpas/src/inc/dateutil.inc	(revision 35221)
+++ packages/rtl-objpas/src/inc/dateutil.inc	(working copy)
@@ -1282,11 +1282,19 @@
 
 
 Function YearsBetween(const ANow, AThen: TDateTime): Integer;
+var
+  yy, mm, dd: Word;
 begin
-  Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerYear);
+  if (ANow >= -DateDelta) and (AThen >= -DateDelta) and
+     (ANow <= MaxDateTime) and (AThen <= MaxDateTime) then
+  begin
+    PeriodBetween(ANow, AThen, yy , mm, dd);
+    Result := yy;
+  end
+  else
+    Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+TDateTimeEpsilon)/ApproxDaysPerYear);
 end;
 
-
 Function MonthsBetween(const ANow, AThen: TDateTime): Integer;
 begin
   Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerMonth);
yearsbetween.diff (913 bytes)

Bart Broersma

2017-01-30 19:01

reporter  

yearsbetween-commented.diff (1,152 bytes)
Index: packages/rtl-objpas/src/inc/dateutil.inc
===================================================================
--- packages/rtl-objpas/src/inc/dateutil.inc	(revision 35221)
+++ packages/rtl-objpas/src/inc/dateutil.inc	(working copy)
@@ -1280,13 +1280,25 @@
     Result:=Result+0.5;
 end;
 
-
+{
+  For years between 0001/01/01 and 9999/12/31 the function is exact,
+  for other years the funcion may be off by 1 if mm-dd in both dates
+  are the same. This is due to the use of ApproxDaysPerYear.
+}
 Function YearsBetween(const ANow, AThen: TDateTime): Integer;
+var
+  yy, mm, dd: Word;
 begin
-  Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerYear);
+  if (ANow >= -DateDelta) and (AThen >= -DateDelta) and
+     (ANow <= MaxDateTime) and (AThen <= MaxDateTime) then
+  begin
+    PeriodBetween(ANow, AThen, yy , mm, dd);
+    Result := yy;
+  end
+  else
+    Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+TDateTimeEpsilon)/ApproxDaysPerYear);
 end;
 
-
 Function MonthsBetween(const ANow, AThen: TDateTime): Integer;
 begin
   Result:=Trunc((Abs(DateTimeDiff(ANow,AThen))+HalfMilliSecond)/ApproxDaysPerMonth);

Bart Broersma

2017-01-30 19:01

reporter   ~0097824

Attached 2 diffs. One without, one with comments in the source.

Michael Van Canneyt

2017-02-11 16:20

administrator   ~0098106

As for MonthsBetween, the approximate value is documented and Delhpi compatible:
http://www.freepascal.org/docs-html/rtl/dateutils/yearsbetween.html

Same solution as for MonthsBetween: I have added an AExact : Boolean = False parameter.

Issue History

Date Modified Username Field Change
2017-01-15 19:38 Bart Broersma New Issue
2017-01-15 19:41 Bart Broersma Note Added: 0097504
2017-01-15 19:42 Bart Broersma Note Edited: 0097504 View Revisions
2017-01-15 19:59 Bart Broersma Note Added: 0097505
2017-01-15 20:09 Bart Broersma Note Added: 0097506
2017-01-15 22:40 wp Note Added: 0097509
2017-01-15 22:41 wp Note Edited: 0097509 View Revisions
2017-01-19 18:21 Bart Broersma Note Added: 0097599
2017-01-19 18:42 Bart Broersma Note Added: 0097600
2017-01-19 18:42 Bart Broersma Note Edited: 0097600 View Revisions
2017-01-19 18:43 Bart Broersma Note Edited: 0097599 View Revisions
2017-01-21 15:01 Bart Broersma Note Added: 0097618
2017-01-21 17:43 wp Note Added: 0097622
2017-01-22 00:57 Bart Broersma Note Added: 0097625
2017-01-22 09:59 wp Note Added: 0097628
2017-01-22 12:22 Bart Broersma Note Added: 0097631
2017-01-29 11:48 Michael Van Canneyt Assigned To => Michael Van Canneyt
2017-01-29 11:48 Michael Van Canneyt Status new => assigned
2017-01-29 11:48 Michael Van Canneyt Note Added: 0097775
2017-01-30 19:01 Bart Broersma File Added: yearsbetween.diff
2017-01-30 19:01 Bart Broersma File Added: yearsbetween-commented.diff
2017-01-30 19:01 Bart Broersma Note Added: 0097824
2017-02-11 16:20 Michael Van Canneyt Fixed in Revision => 35423
2017-02-11 16:20 Michael Van Canneyt Note Added: 0098106
2017-02-11 16:20 Michael Van Canneyt Status assigned => resolved
2017-02-11 16:20 Michael Van Canneyt Fixed in Version => 3.1.1
2017-02-11 16:20 Michael Van Canneyt Resolution open => fixed
2017-02-11 16:20 Michael Van Canneyt Target Version => 3.2.0
2017-02-11 18:05 Bart Broersma Status resolved => closed