[m-rev.] for review: fix a bug in calendar.duration_between/2

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat Jun 27 14:57:31 AEST 2026



On Sat, 27 Jun 2026 14:16:02 +1000, Julien Fischer <jfischer at opturion.com> wrote:
> Is anyone intending to review this one?   I will commit it tomorrow in
> the absence
> of a review.

I looked at this when you first posted it, but I did not want to review it,
because it involves the reason why I am skeptical of the utility of time differences
when expressed in variable units such as months: the fact that what e.g. "one month"
difference means is not obvious, and reasonable people can disagree about it.
I know that people do use such differences in real life, but they usually do so
in contexts where being off by a day or two does not matter. I hoped Peter
would offer a review, but ...

> > +    % greedy_subtract_descending(OriginalOrder, DateA, DateB, Duration):
> > +    %

What is the utility of having one predicate do both original orders?
It is harder to see in a diff, but I have the impression that the predicate body
has significantly more code that differs between two cases than common code.
If you had the two cases in different predicates, you could use variable names
that would be MUCH more descriptive than Date[AB], and readers wouldn't have to
juggle the two meanings of each variable name in their heads.

Your logic seems ok to me, but, due to the issue above, I am not confident
in saying that. If you post a version that I find more readable, I could review
that later today.

Zoltan.


More information about the reviews mailing list