[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