[m-rev.] for review: document the duration/0 type more thoroughly

Zoltan Somogyi zoltan.somogyi at runbox.com
Fri Feb 13 19:16:04 AEDT 2026



On Fri, 13 Feb 2026 12:15:01 +1100, Julien Fischer <jfischer at opturion.com> wrote:
> diff --git a/library/calendar.m b/library/calendar.m
> index 868706c23..fb7586d30 100644
> --- a/library/calendar.m
> +++ b/library/calendar.m
> @@ -186,6 +186,38 @@
>      % seconds and microseconds. Internally a duration is represented
>      % using only months, days, seconds and microseconds components.
>      %
> +    % The year and month components are context-dependent units.
> +    % Their actual length in absolute time varies depending on the specific
> +    % dates to which they are applied:
> +    %   - A year is treated as 12 months
> +    %   - A month's length varies (28-31 days depending on the month)
> +    %   - A year's length varies (365 or 366 days depending on leap years)
> +    %
> +    % This context-dependent behavior is intentional and lets durations
> +    % represent calendar-based time periods accurately. For example:
> +    %   - Adding "1 month" to January 15 gives February 15 (31 days later)
> +    %   - Adding "1 month" to February 15 gives March 15 (28 or 29 days later)
> +    %   - Adding "1 year" to February 29, 2020 gives February 28, 2021
> +    %
> +    % In contrast, days, hours, minutes, seconds and microseconds are
> +    % fixed-length units:
> +    %   - 1 day = 24 hours
> +    %   - 1 hour = 60 minutes
> +    %   - 1 minute = 60 seconds
> +    %   - 1 second = 1,000,000 microseconds
> +    %
> +    % Note on leap seconds: Although individual dates can represent times
> +    % with leap seconds (seconds in the range 60-61), this module ignores
> +    % leap seconds when computing durations. When calculating the duration
> +    % between two dates, any leap seconds that occurred in that interval
> +    % are not accounted for. This means a "day" in a duration is always
> +    % treated as exactly 86,400 seconds, even though actual UTC days
> +    % containing leap seconds may be 86,401 seconds.
> +    %
> +    % If you need a fixed absolute time period that is independent of
> +    % calendar context, then use only the day, hour, minute, second and
> +    % microsecond components.
> +    %
>  :- type duration.

That is a clear explanation.

It does look a bit out-of-place, since users can "use only the day ... etc"
only when calling init_duration, so that part of the comment could
maybe be better placed on that function. Whether you want to move it
or not, the diff is fine, and you can commit it.

>      % Duration components.
> @@ -209,7 +241,7 @@
>  :- func microseconds(duration) = microseconds.
> 
>      % init_duration(Years, Months, Days, Hours, Minutes,
> -    %   Seconds, MicroSeconds) = Duration.
> +    %   Seconds, MicroSeconds) = Duration:
>      % Create a new duration. All of the components should either be
>      % non-negative or non-positive (they can all be zero).
>      %

This is not a new issue, but

- there should be a blank line before "% Create ...", and

- "All of the components should either be non-negative or non-positive
  (they can all be zero)" is massively misleading. This is because the usual
  reading of its meaning is that the "non-negative or non-positive or zero"
  constraint (which is not a constraint it all) applies to each argument
  *individually*. What both the code and sanity require is that either
  *all* args are non-negative, or *all* args are non-positive.

How about:

"If the duration is supposed to go forward in time, then all arguments
must be either positive or zero.

If the duration is supposed to go backward in time, then all arguments
must be either negative or zero.

If the argument list contains both positive and negative values,
this function will throw an exception".

Zoltan.





More information about the reviews mailing list