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

[Peter Wang]

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.

> > @@ -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

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.

> > +:- 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?

> @@ -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.)

Peter