[m-rev.] for review: improve the documentation for array.fetch_items
jfischer at opturion.com
Mon Dec 24 15:50:53 AEDT 2018
On Mon, 24 Dec 2018, Zoltan Somogyi wrote:
> 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 ...
I am; fixed.
> Aren't there any tests that would have caught these?
Only the indirect one in tests/hard_coded/array_test2; I will add a more
> 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.
True, although the old version had that behaviour as well. I've swapped
the order of the tests about. Here's a diff to the NEWS file
announcing the change.
diff --git a/NEWS b/NEWS
index 60f3746..db0738a 100644
@@ -123,6 +123,11 @@ Changes that may break compatibility:
* We have changed the semantics of array.shrink/3 and array.resize/4 so that
they throw an exception if their first argument is negative.
+* We have changed the semantics of array.fetch_times/4 so that it always
+ throws an exception if its second or third argument is out of bounds.
+ Previously, it would return an empty if list if its third argument was less
+ than the second even if one, or both, of these arguments was out of bounds.
Changes to the Mercury language:
* We have added a new primitive type, uint, which is an unsigned integer type
More information about the reviews