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@eldwood.com)