On Thu, Dec 19, 2019 at 1:12 PM Krystian Stasiowski via Boost < boost@lists.boost.org> wrote:
My mini-review will focus on the container_interface.
On Tue, Dec 10, 2019 at 7:21 PM Barrett Adair via Boost < boost@lists.boost.org> wrote:
- Your name
Krystian Stasiowski
Thanks for the review, Krystian.
- What is your evaluation of the library's * Design?
Overall, its pretty solid. One notable thing that comes to mind is that this should be made SFINAE friendly, ie disable certain overloads when the required functions are not defined in the derived class.
This is the case, within the limits of the sequence container concept. That is, if there is some operation (say, insert()) that is required by all sequence containers, it would be inappropriate to SNIFAE-away such an operation if the derived class does not support it. To do so would mean that you end up with a container_interface-derived type that is a sequence container. However, there are optional sets of requirements for sequence containers (the reversible container requirements are one example). For those optional requirements, I believe that they are all SFINAE-friendly. If you find a counterexample, that's a bug; please report it.
Additionally, providing a destructor is a big no-no in my opinion, and could lead to elements having their destructor called twice. Infact, the documentation says that the user must provide a destructor that destroys all the elements of the container, but the destructor provided by the container_interface clears the container anyways, which leads to UB.
Agreed. As stated in another thread, I'll remove the destructor implementation.
* Implementation?
This implementation is pretty solid.
* Documentation?
The documentation is _alright_. One thing I noticed is there is no cohesive list of which functions are provided by the interface when others are implemented by the user. It makes it quite a pain to navigate and even confusing.
Could you point specifically to the part of the docs that you find hard to follow? There is already this sort of cohesive list in the case of container_interface. For the iterators, there is not. In both cases, the list of operations you end up with is well documented on cppreference,com and eel.is, because it comes from the standard. Zach