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

[Zoltan Somogyi]

On Sun, 28 Jun 2026 00:20:57 +1000, Julien Fischer <jfischer at opturion.com> wrote:

> On Sat, 27 Jun 2026 at 14:57, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
> 
> > > > +    % greedy_subtract_descending(OriginalOrder, DateA, DateB, Duration):
> > > > +    %
> >
> > What is the utility of having one predicate do both original orders?
> 
> At this point, there is arguably no utility.
> 
> > 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.
> 
> The attached diff addresses the above points.

The updated diff is fine, and more readable (at least to me) than the original.

One minor point about this code, and its mirror image:

+    add_duration(init_duration(0, MonthsUpperBound, 0, 0, 0, 0, 0),
+        Earlier, Candidate),
+    ( if compare((>), Candidate, Later) then
+        Months = MonthsUpperBound - 1
+    else
+        Months = MonthsUpperBound
+    ),
+    add_duration(init_duration(0, Months, 0, 0, 0, 0, 0), Earlier, Landing),

If the condition fails, both calls to add_duration do the same job.
I would assign Candidate to Landing in the then-part, and move
the second add_duration to the else part, even though I know
this code is not likely to be perf critical.

Zoltan.