[m-rev.] for review: improve the documentation for array.fetch_items

Zoltan Somogyi zoltan.somogyi at runbox.com
Mon Dec 24 15:35:31 AEDT 2018



On Mon, 24 Dec 2018 04:25:16 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
> -    % fetch_items takes an array and a lower and upper index,
> -    % and places those items in the array between these indices into a list.
> -    % It is an error if either index is out of bounds.
> +    % fetch_items(Array, Lo, Hi, List):
> +    % Returns a list containing the items in the array with index in the range
> +    % Lo..Hi (inclusive) in the same order that they occurred in the array.
> +    % If Hi < Lo then the empty list is returned.

I would reword as "Returns an empty list if Hi < Lo", to fit in better
with neighbouring sentences. And I would replace "(inclusive)" with
"(both inclusive)" for clarity.

> +    else if in_bounds(Array, Low) then
> +        arg_out_of_bounds_error(Array, "second", "fetch_items", Low)
> +    else if in_bounds(Array, High) then
> +        arg_out_of_bounds_error(Array, "third", "fetch_items", High)

??? I think you are missing a "not" in each of those conditions ...
Aren't there any tests that would have caught these?

And another possible problem: when Hi < Lo but one or both are out of
bounds, this code returns an empty list, instead of throwing an exception.

Zoltan.


More information about the reviews mailing list