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
- Your knowledge of the problem domain
I enjoy implementing generic containers, and dabble in the subject.
- Whether you believe the library should be accepted into Boost (be clear about this)
I vote to ACCEPT.
- 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. 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.
* 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.
* Usefulness?
I see this being useful for implementations of containers that don't have to be extremely optimized. For example, the implementation of `clear` provided by the implementation is not optimal for trivial types
- Did you attempt to use the library? If so: * Which compiler(s)?
MSVC.
* What was the experience? Any problems?
I would say that my experience with the library was quite good. To test it out, I opted to rewrite Vinnie's boost::json::array container using container_interface to see how much it would really simplify writing containers. It mostly consisted of removing code as expected.
- How much effort did you put into your evaluation of the review?
I spent about an hour and a half messing with the implementation.