Re: [review] The review of boost::container::hub starts! April 16 - April 26

Hello,
I hope everyone is doing well.

My review of `hub` will cover: Documentation, Design and the implementation of the hub container.

## DESIGN:

- The presence of pre-conditions, a single container, and free functions is a big plus in the design, this lets the developer use the library without too much hurdle on the development side.
- Speaking of development the presence of a NATVIS file is also a big plus in terms of debugging for Visual Studio. (IIRC a lot of users of Boost develop on Windows)
- There is a clear cause and effect in the design as well, easy to reason about and solve problems upon.
- The usage of known requirements like CopyInsertable in the design is also a big plus to avoid technical debt.

## DOCUMENTATION:

- The format of the documentation consists of the function signature, preconditons, snyposis, and effects. It is another plus for users wanting to get started using the library.
- The addition of equivalent examples is also a plus for the library, knowing what else can be done is as important as the solution.

## IMPLEMENTATION:

- It has clean free functions like erase_if and is easy to read, which fits what the library `container` library should do.

```cpp
template<typename T, typename Allocator, typename Predicate>
typename hub<T, Allocator>::size_type
erase_if(hub<T, Allocator>& x, Predicate pred)
{
  using hub_container = hub<T, Allocator>;
  using size_type = typename hub_container::size_type;
  using block = typename hub_container::block;

  auto s = x.size_;
  for(auto pbb = x.blist.next; pbb != x.blist.header(); ) {
    auto pb = x.static_cast_block_pointer(pbb);
    pbb = pb->next;
    BOOST_CONTAINER_HUB_PREFETCH_BLOCK(pbb, block);
    auto mask = pb->mask;
    do {
      auto n = hub_detail::unchecked_countr_zero(mask);
      if(pred(pb->data()[n])) x.erase_impl(pb, n);
      mask &= mask - 1;
    } while(mask);
  }
  return (size_type)(s - x.size_);
}
```
Fig 1: erase_if implementation (https://github.com/joaquintides/hub/blob/develop/include/boost/container/hub.hpp)

- The design pattern is respected per design guidelines and there is a separation of concerns.
- There is a clear consideration for MSVC and other compilers, which is a big plus for adoption.

## EFFORT:

I went and looked at the documentation, implementation and overall structure of the repository.

## COMMENTS:

- The transparency about `BOOST_CONTAINER_HUB_PREFETCH` is really valuable, it's wroth finding more cases like this in the future and documenting them.

## CHOICE:

I vote to ACCEPT `hub` to be merged into `container`. It clearly has a lot of upsides (including HFT, per the author's examples)
_______________________________________________
Boost mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://lists.boost.org/mailman3/lists/boost.lists.boost.org/
Archived at: https://lists.boost.org/archives/list/[email protected]/message/BAXPANROBL2OWB57HJFWVMX7A75MX7J2/