[m-rev.] for post-commit review: adjust documentation of the ranges module

[Zoltan Somogyi]

On 2024-03-30 14:30 +11:00 AEST, "Julien Fischer" <jfischer at opturion.com> wrote:
> On Sat, 30 Mar 2024, Zoltan Somogyi wrote:
> 
>> On 2024-03-30 02:22 +11:00 AEDT, "Julien Fischer" <jfischer at opturion.com> wrote:44
>>> --- a/library/ranges.m
>>> +++ b/library/ranges.m
>>> @@ -34,70 +34,71 @@
>>>      %
>>>  :- func empty = ranges.
>>>
>>> -    % is_empty(Set):
>>> -    % Succeeds iff Set is the empty set.
>>> +    % is_empty(Set) is true iff Set is the empty set.
>>>      %
>>>  :- pred is_empty(ranges::in) is semidet.
>>
>> This comment is not unique to this diff, but I wonder whether
>> using "iff" as a shorthand helps more people (by being brief)
>> than it hurts (by baffling anyone who does not know what it
>> stands for). Given that the positive effect is small while the
>> negative effect is large, I think it would be better to phase out
>> its use.
> 
> I have no objection to that.

Quite late, but the attached committed diff does this.

(I am cleaning out my inbox.)
 
>> If this diff is here, that means that this module does not *order*
>> its exported predicates the same way as the other set predicates do.
>> You may want to fix that in your next diff.
> 
> No, it doesn't order its exported predicates as per the other set
> modules. I intend to fix that in subsequent change.

I don't think this has been done yet.
 
>>> -    % Shift a range by const C.
>>> +    % Shift a range by a constant C.
>>>      %
>>>  :- func shift(ranges, int) = ranges.
>>> -    % Dilate a range by const C.
>>> +    % Dilate a range by a constant C.
>>>      %
>>>  :- func dilation(ranges, int) = ranges.
>>> -    % Contract a range by const C.
>>> +    % Contract a range by a constant C.
>>>      %
>>>  :- func contraction(ranges, int) = ranges.
>>>
>>
>> I had no idea what these functions do, and still don't.
>> And the function names don't help.
> 
> I know; I will deal with these after I have some more extensive testing
> of this module in place; among other things contraction appears to
> violate the invariants of the ranges types.

And these functions don't have any useful documentation yet either.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.iff
Type: application/octet-stream
Size: 173640 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20250608/cdab4828/attachment-0001.obj>