[m-rev.] for review: conversion from array/1 to array2d/1

Julien Fischer jfischer at opturion.com
Fri Nov 30 15:54:44 AEDT 2018


On Thu, 29 Nov 2018, Peter Wang wrote:

> On Thu, 29 Nov 2018 06:06:14 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 88d36a7..8463bba 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -474,10 +474,11 @@ Changes to the Mercury standard library:
>>>     - least_index/1
>>>     - greatest_index/1
>>>
>>> -* The following predicates have been added to the array2d module:
>>> +* The following predicates and functions have been added to the array2d
>>> module:
>
> Overlong line.

Fixed.

>>> @@ -46,7 +46,7 @@
>>>  :- func init(int, int, T) = array2d(T).
>>>  :- mode init(in, in, in) = array2d_uo is det.
>>>
>>> -    % array2d([[X11, ..., X1N], ..., [XM1, ..., XMN]]) constructs a array2d
>>> +    % array2d([[X11, ..., X1N], ..., [XM1, ..., XMN]]) constructs an array2d
>>>     %  of size M * N, with the special case that bounds(array2d([]), 0, 0).
>>>     %
>>>     %  An exception is thrown if the sublists are not all the same length.
>>> @@ -54,6 +54,20 @@
>>>  :- func array2d(list(list(T))) = array2d(T).
>>>  :- mode array2d(in) = array2d_uo is det.
>>>
>>> +    % A synonym for the above.
>>> +    %
>>> +:- func from_lists(list(list(T))) = array2d(T).
>>> +:- mode from_lists(in) = array2d_uo is det.
>>> +
>>> +    % from_array(M, N, Array) constructs an array2d of size M * N whose
>>> +    % elements are taken from Array.  The elements in Array are added to
>>> +    % the array2d in row-major order (see above).
>>> +    % Throws an exception if M < or N < 0, or if the number of elements in
>>> +    % Array does not equal M * N.
>
> M < 0

Fixed.

>
> I suggest something like:
>
>    from_array(M, N, Array) constructs an array2d of size M * N
>    where the elements are taken from Array in row-major order,
>    i.e. the element at row I column J is taken from Array at index
>    (I * N + J). Indices start from zero.

Ok, I've used that wording.

>>> +:- func from_array(int, int, array(T)) = array2d(T).
>>> +:- mode from_array(in, in, array_di) = array2d_uo is det.
>
> Would from_array_row_major be clearer, or too much of a mouthful?

Too much of a mouthful.  (We might add it as synonym if
from_array_col_major were ever added, but I think only then.)

>> @@ -80,7 +89,7 @@
>>  :- mode in_bounds(in,       in,  in ) is semidet.
>>
>>     %  array2d([[X11, ..., X1N], ..., [XM1, ..., XMN]]) ^ elem(I, J) = X
>> -    % where X is the J+1th element of the I+1th row (that is, indices
>> +    % where X is the J+1'th element of the I+1'th row (that is, indices
>>     %  start from zero.)
>
> Ugh, I suggest something like:
>
>    Array2d ^ elem(I, J) = X:
>    Returns the element X at row I column J of Array2d.
>    Indices start from zero.
>
> I think most of the the [[X11, ... ], ...] stuff in the documentation
> should just be replaced by variable names. Actually, much of the
> documentation in the module is unnecessary cryptic. (You don't have to
> fix it now.)

I take no responsibility for residual Becketisms in the documentation of
this module! ;-)

(I'll look into adjusting the documentation separately.)

Thanks for that.

Cheers,
Julien.


More information about the reviews mailing list