[m-rev.] for post-commit review: organize array.m, bt_array.m and pprint.m

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat Jun 11 11:08:32 AEST 2022


2022-06-10 19:55 GMT+10:00 "Julien Fischer" <jfischer at opturion.com>:
> I would not prefix the names of all the operations in this module with "ra_list_".
> It makes the names unwieldy and also complicates swapping between ra_lists and
> other sequence types such as cords or standard lists.

I have deleted the ra_list_ prefix, except from ra_list_to_list, where
it does not play the role of a prefix.

>> +
>> +    % Return an empty random access list.
>> +    %
>> +:- pred ra_list_nil(ra_list(T)::uo) is det.
> 
> For consistency with elsewhere, I suggest you call this init.

Done.

> This module lacks many of the standard operations I would expect to exist on a
> container style data structure.  Some of these can be added as required, but
> the following should be added now:
> 
>    - is_empty / is_not_empty
>    - singleton
>    - is_singleton
>    - length
>    - foldl
>    - foldr
>    - map

I will add these in my next diff.
 
>> +:- pred ra_list_head(ra_list(T)::in, T::out) is semidet.
> 
> This is a function in the list module (it probably shouldn't be) and is named
> get_first in the cord module.

I will ensure that all three modules have two semidet preds named head
and get_first.

> The names of these operations differ from the corresponding ones in the list module.
> (That module also offers both zero- and one-based indexing.)

Fixed.

> The diff is otherwise fine.

Thanks for the review.

Zoltan.


More information about the reviews mailing list