[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