Re: [review] [STLInterfaces] STLInterfaces review starts today

From: Rainer Deyke (rainerd_at_[hidden])
Date: 2019-12-16 16:49:05

On 11.12.19 01:21, Barrett Adair via Boost wrote:
> Dear Boost,
> 
> The formal review of Zach Laine's STLInterfaces library begins now, and
> will run through December 19. Please participate in this review if you can.
> To submit a review, please reply to this email with the following
> information:
> - Your name

Rainer Deyke

> - Your knowledge of the problem domain

I'm hardly an expert on containers or iterators, but I've written a 
STL-compatible container or two.

> - Whether you believe the library should be accepted into Boost (be clear
> about this)

Yes, it should be included.

> In addition, you are strongly encouraged to answer the following questions:
> - What is your evaluation of the library's
>    * Design?

Seems fine.  I have a few nits to pick, but nothing that would hold up 
the inclusion of the library in Boost.

Nit 1: I don't like 'bool Contiguous = discontiguous'.  If a symbolic 
name like discontiguous is provided, then it should be part of an enum 
and not a plain boolean.  This prevents the name from being misused in 
other contexts.

Nit 2: It seems to me that, with a bit of effort, container_interface 
could do much more to make containers easier to write.  For example:
   - X::iterator can be inferred from begin().
   - X::reference is just X::value_type &.
   - operators < and == can be provided, since the requirements of these 
functions already suggest a default implementation.  The user can still 
override the provided implementation if a more efficient implementation 
is possible.  If it is for some reason desirable to create a container 
without operator <, an additional policy argument could be added to 
container_interface.

If there's a good reason for container_interface not to provide these, 
then this reason should be documented.

>    * Implementation?

Didn't look at it.  I assume that it's fine, or that any issues with it 
are too subtle for me to find.

>    * Documentation?

There are two types of tables with different columns in the 
container_interface documentation: those that document user requirements 
and those that document the functionality provided by 
container_interface.  This can make reading difficult.  If I just want 
to write a container I am only interested in the user requirements, so I 
have to skip every other table.  If I am searching for specific 
functions (using the text search functionality of my browser) to see if 
container_interface provides it, I have to mentally switch between the 
two table types.  It might be better to either merge the two table 
types, or to group all tables of the same type together.

I think it should be clarified that although container_interface 
provides no help in making containers allocator-aware, it can still be 
used for writing allocator-aware containers.  It is simply up to the 
user to provide all allocator-related functionality.  (At least I assume 
that's the case?)

>    * Tests?

Didn't look.

>    * Usefulness?

Fairly useful.  I don't see myself using it often, it would definitely 
be useful for the few times when I do need to write a custom container 
or a custom iterator.

> - Did you attempt to use the library?

No.

> - Do you like the name container_interface?  sequence_container_interface
> is more precise, but seems a bit long.

Since I'm only going to be writing out the full name of the template 
once per container, I think I prefer the longer, more precise name.

> - Would you use something like container_interface, but for associative
> containers?  If so, should it assume a node-based interface, a la std::map
> and std::set?  This assumption would preclude alternative associative
> container-like types, such as flat_map.

I can see the usefulness of such a template, although I have no 
immediate use for it.  I would prefer that it not assume a node-based 
interface.

-- 
Rainer Deyke (rainerd_at_[hidden])