[m-rev.] for review: bt_array__map implementation; bsearch last argument type qn

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Mar 15 17:38:31 AEDT 2002


On 15-Mar-2002, Peter Moulder <pmoulder at csse.monash.edu.au> wrote:
> It's nice to be able to switch back and forth between array and bt_array
> relatively easily.

Well, the obvious big difference is that `array' uses unique modes
whereas `bt_array' doesn't.  This means that switching between the
two can never be made entirely painless.

> The minimum index thing is one obvious difference
> (all bt_array creation methods take a Low argument; no array creation
> methods do).  This isn't a huge issue, just adding to the number of
> changes to be done (though beware the High/Size distinction).

This is a case of different design goals leading to different interfaces.
For `array', efficiency is very important, so we don't want to pay
the overhead of a `Low' argument.  For `bt_array', that cost is
not important, and so the increased expressiveness is more important.

> There's also a question on bsearch's last argument and determinism,
> whether they should be maybe(int)/det or int/semidet.  It looks as if
> it was originally int/semidet in both array and bt_array, and then
> array__bsearch was changed (1.32) but not bt_array__bsearch.

Using `semidet' is more natural, but it doesn't work with unique modes.
bt_array uses what is more natural, but array needs to use what will
work with unique modes.

> Both modules have a stability comment of medium-low.  Should
> bt_array__bsearch be changed to maybe(int)/det ?

I could live with that.

> Estimated hours taken: 4
> 
> library/bt_array.m:
> 	New `bt_array__map/3' predicate and `bt_array__map/2' function.
> 	Documentation additions & changes.
> tests/general/array.m:
> tests/general/array.exp:
> 	Test array__map and bt_array__map predicates.

> Index: library/bt_array.m
> @@ -170,9 +170,23 @@
>  	% order.  Fails if the element is not present.  If the
>  	% element to be found appears multiple times, the index of
>  	% the first occurrence is returned.
> +	% (Note: the last argument is an int rather than maybe(int) as in
> +	% array__bsearch.  XXX: The discrepancy should probably be removed.
> +	% Both modules have Stability: medium-low.  It appears that int/semidet
> +	% was the original behavior, based on the `Fails if ...' comment still
> +	% present in array__bsearch.  So should probably change this to
> +	% maybe(int)/det.)

Delete the last four lines of this comment.

> @@ -552,8 +595,13 @@
> +	I >= 0 ->
> +		ra_list_lookup_2(I, List, X)
> +	;
> +		fail.

All if-then-elses should be in parentheses.

Otherwise that looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list