On Mon, Dec 16, 2019 at 10:49 AM Rainer Deyke via Boost < boost@lists.boost.org> wrote:
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
Thanks for the review, Rainer!
- 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.
I understand this completely, and usually I do exactly this, for exactly this reason. However, in this case, the Contiguous template param is a hack that bridges us to C++20 code, which knows how to figure this out in a standard way. To make an enum for this implies a primacy that this template parameter does not deserve. That being said, if others find the bool objectionable too, I'm not opposed to changing it.
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 &.
These cannot be provided by the CRTP base in the general case, because they may be used in the body of the derived class, and they will not be visible early enough if they come from the base class. I'll document this. - 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.
These are all provided; I think you just missed them. Zach