[m-rev.] for review: add array.swap/4 and array.unsafe_swap/4

Zoltan Somogyi zoltan.somogyi at runbox.com
Mon Nov 12 17:30:11 AEDT 2018



On Mon, 12 Nov 2018 17:20:04 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
> +    % swap(I, J, !Array):
> +    % Swap the item in the I'th position with the item in the J'th position.
> +    % Throws an exception if either of I or J is out-of-bounds.
> +    %
> +:- pred swap(int, int, array(T), array(T)).
> +:- mode swap(in, in, array_di, array_uo) is det.
> +
> +    % As above, but omit the bounds check.

I would say checks (plural).

> @@ -350,6 +362,7 @@
>   %:- mode copy(array_ui) = array_uo is det.
>   :- mode copy(in) = array_uo is det.
> 
> +
>       % resize(Size, Init, Array0, Array):
>       % The array is expanded or shrunk to make it fit the new size `Size'.
>       % Any new entries are filled with `Init'. Throws an exception if

?

> @@ -2141,9 +2154,9 @@ fill_range(Item, Lo, Hi, !Array) :-
>       ( if Lo > Hi then
>           unexpected($pred, "empty range")
>       else if not in_bounds(!.Array, Lo) then
> -        out_of_bounds_error(!.Array, Lo, "fill_range")
> +        arg_out_of_bounds_error(!.Array, "second", Lo, "fill_range")
>       else if not in_bounds(!.Array, Hi) then
> -        out_of_bounds_error(!.Array, Hi, "fill_range")
> +        arg_out_of_bounds_error(!.Array, "third", Hi, "fill_range")
>       else
>           do_fill_range(Item, Lo, Hi, !Array)

The arg order of arg_out_of_bounds seems strange: the arg number and
the pred name logically belong next to each other, but you put the index
between them.

> +test_swap(Elems, I, J, !IO) :-
> +    io.write_string("================\n", !IO),
> +    array.from_list(Elems, Array0),
> +    io.write_string("Array0 = ", !IO),
> +    io.write_line(Array0, !IO),
> +    io.format("Swap: %d <-> %d\n", [i(I), i(J)], !IO),
> +    ( try [] (
> +        array.swap(I, J, Array0, Array)
> +    ) then
> +        io.write_string("Array = ", !IO),
> +        io.write_line(Array, !IO)
> +    catch index_out_of_bounds(S) ->
> +        io.write_string("EXCEPTION: ", !IO),
> +        io.write_line(S, !IO)
> +    ).

The formatting of the try block is strange. The close paren
before "then" looks like closing the whole thing, because it
aligns with the open paren before "try", not the open paren
after "try".

Otherwise, the diff is fine.

Zoltan.


More information about the reviews mailing list