[PATCH/RFH] date: avoid "X years, 12 months" in relative dates

[PATCH/RFH] date: avoid "X years, 12 months" in relative dates

[Jeff King]

When relative dates are more than about a year ago, we start
writing them as "Y years, M months".  At the point where we
calculate Y and M, we have the time delta specified as a
number of days. We calculate these integers as:

  Y = days / 365
  M = (days % 365 + 15) / 30

This rounds days in the latter half of a month up to the
nearest month, so that day 16 is "1 month" (or day 381 is "1
year, 1 month").

We don't round the year at all, though, meaning we can end
up with "1 year, 12 months", which is silly; it should just
be "2 years".

The implementation is crude but effective: we check for this
silly case with a conditional and tweak the values:

  if (M == 12) {
	  Y++;
	  M = 0;
  }

It's tempting to look for a purely numerical solution, like
just rounding better. But it is made difficult by the fact
that our approximation of a month is "30 days", which means
that a year does not actually contain 12 of our approximate
months (there is a 5-day discrepancy).

So you could do:

  Y = (days + 15) / 365

which will round up the year if we are in the last 15 days.
And then you would make a matching tweak for months:

  M = (days + 15) % 365 / 30;

so that they both round at exactly the same day.

But that doesn't quite work. Day 715 will be properly
rounded to "2 years, 0 months", which is what we want. But
days 710 through 714 remain at "1 year, 12 months", due to
the 5-day discrepancy.

Another attempt would be to write it as the much more
readable:

  total_months = (days + 15) / 30;
  Y = total_months / 12;
  M = total_months % 12;

But our 5-day discrepancy hits us again, and this time much
worse, since the error compounds with each year. On day
3650, we should be exactly at "10 years", but the above puts
us at "10 years, 2 months".

Signed-off-by: Jeff King <peff@peff.net>
---
So I thought this patch would take me about 2 minutes to write, and (as
you can probably guess from the length of the commit message), it turned
out much more complex. I'd be curious to see if anybody has a numerical
solution using integers.

With floating point, it's pretty simple:

  double one_month = 365 / 12;
  unsigned long total_months = ((double)days + (one_month / 2)) / one_month;
  unsigned long years = total_months / 12;
  unsigned long months = total_months % 12;

but I'm beginning to think it's not possible to make a simple formula
with just integers. Which maybe means we should just use floating point
here. Or maybe somebody can prove me to be an idiot.

 date.c          |    5 +++++
 t/t0006-date.sh |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/date.c b/date.c
index 00f9eb5..3dfade2 100644
--- a/date.c
+++ b/date.c
@@ -132,6 +132,11 @@ const char *show_date_relative(unsigned long time, int tz,
 		unsigned long years = diff / 365;
 		unsigned long months = (diff % 365 + 15) / 30;
 		int n;
+
+		if (months == 12) {
+			years++;
+			months = 0;
+		}
 		n = snprintf(timebuf, timebuf_size, "%lu year%s",
 				years, (years > 1 ? "s" : ""));
 		if (months)
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 1d4d0a5..f87abb5 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -25,6 +25,7 @@ check_show 37500000 '1 year, 2 months ago'
 check_show 55188000 '1 year, 9 months ago'
 check_show 630000000 '20 years ago'
 check_show 31449600 '12 months ago'
+check_show 62985600 '2 years ago'
 
 check_parse() {
 	echo "$1 -> $2" >expect
-- 
1.7.5.rc2.4.gc3ad3

[PATCH] date: avoid "X years, 12 months" in relative dates

[Michael J Gruber]

When relative dates are more than about a year ago, we start
writing them as "Y years, M months".  At the point where we
calculate Y and M, we have the time delta specified as a
number of days. We calculate these integers as:

  Y = days / 365
  M = (days % 365 + 15) / 30

This rounds days in the latter half of a month up to the
nearest month, so that day 16 is "1 month" (or day 381 is "1
year, 1 month").

We don't round the year at all, though, meaning we can end
up with "1 year, 12 months", which is silly; it should just
be "2 years".

Implement this differently with months of size

  onemonth = 365/12

so that

  totalmonths = (long)( (days + onemonth/2)/onemonth )
  years = totalmonths / 12
  months = totalmonths % 12

In order to do this without floats, we write the first formula as

  totalmonths = (days*12*2 + 365) / (365*2)

Tests and inspiration by Jeff King.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 date.c          |    5 +++--
 t/t0006-date.sh |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/date.c b/date.c
index 00f9eb5..cde239c 100644
--- a/date.c
+++ b/date.c
@@ -129,8 +129,9 @@ const char *show_date_relative(unsigned long time, int tz,
 	}
 	/* Give years and months for 5 years or so */
 	if (diff < 1825) {
-		unsigned long years = diff / 365;
-		unsigned long months = (diff % 365 + 15) / 30;
+		unsigned long totalmonths = (diff*12*2 + 365)/(365*2);
+		unsigned long years = totalmonths / 12;
+		unsigned long months = totalmonths % 12;
 		int n;
 		n = snprintf(timebuf, timebuf_size, "%lu year%s",
 				years, (years > 1 ? "s" : ""));
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 1d4d0a5..f87abb5 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -25,6 +25,7 @@ check_show 37500000 '1 year, 2 months ago'
 check_show 55188000 '1 year, 9 months ago'
 check_show 630000000 '20 years ago'
 check_show 31449600 '12 months ago'
+check_show 62985600 '2 years ago'
 
 check_parse() {
 	echo "$1 -> $2" >expect
-- 
1.7.5.rc1.312.g1936c

Re: [PATCH] date: avoid "X years, 12 months" in relative dates

[Jeff King]

On Wed, Apr 20, 2011 at 11:12:11AM +0200, Michael J Gruber wrote:

> Implement this differently with months of size
> 
>   onemonth = 365/12
> 
> so that
> 
>   totalmonths = (long)( (days + onemonth/2)/onemonth )
>   years = totalmonths / 12
>   months = totalmonths % 12
> 
> In order to do this without floats, we write the first formula as
> 
>   totalmonths = (days*12*2 + 365) / (365*2)

Well now I feel like an idiot. Algebra to the rescue.

The extra multiplications introduce the possibility of overflow, but
since the number of days was arrived at by dividing an unsigned long
number of seconds by 86400, we are guaranteed to have room to multiply
by 24. :)

So looks good to me.

-Peff

Re: [PATCH] date: avoid "X years, 12 months" in relative dates

[Michael J Gruber]

Jeff King venit, vidit, dixit 20.04.2011 12:18:
> On Wed, Apr 20, 2011 at 11:12:11AM +0200, Michael J Gruber wrote:
> 
>> Implement this differently with months of size
>>
>>   onemonth = 365/12
>>
>> so that
>>
>>   totalmonths = (long)( (days + onemonth/2)/onemonth )
>>   years = totalmonths / 12
>>   months = totalmonths % 12
>>
>> In order to do this without floats, we write the first formula as
>>
>>   totalmonths = (days*12*2 + 365) / (365*2)
> 
> Well now I feel like an idiot. Algebra to the rescue.

:)

> The extra multiplications introduce the possibility of overflow, but
> since the number of days was arrived at by dividing an unsigned long
> number of seconds by 86400, we are guaranteed to have room to multiply
> by 24. :)

Also, this is inside "if (diff < 1825)", so no matter where diff comes
from, "long" is really long enough.

Cheers,
Michael