Subject: Re: [boost] [review] The review of Boost.DoubleEnded starts today: September 21 - September 30
From: Thorsten Ottosen (tottosen_at_[hidden])
Date: 2017-09-27 19:27:07
Den 26-09-2017 kl. 23:43 skrev Joaquin M López Muñoz via Boost:
> El 21/09/2017 a las 19:38, Thorsten Ottosen via Boost escribió:
>> Dear users and members of Boost,
>>
[snip]
> and mostly utility-level. We already have a bestiary of containers in
> Boost, namely
> Boost.Container, so I think both devector and batch_deque are better
> served if proposed
> as components of this latter libray. Code in boost::double_ended::detail
> can go to / merge
> into boost::container[::detail] --for instance,
> boost::double_ended::detail::allocator_traits
> seems to be redundant with boost::containter::allocator_traits.
This makes sense, but it does require that the author of Boost.Container
is willing to entertain that idea, doesn't it?
>
> REVIEW FOR DEVECTOR
> and violates the "don't pay for what you don't use" principle. I'd
> remove it.
How does it violate that?
> 7. Modulo the points discussed above, the interface of devector follows
> closely that
> of std::vector (with the obvious additions, e.g.
> [push|pop_emplace]_front), which is
> very good. The only interface decisions I'm not fully convinced about is
> capacity/
> resize/reserve/shrink_to_fit without "front" or "back" qualification:
What about generic code that can be used with either a vector or devector?
> Â - capacity() seems to be (from what I can infer reading the docs
> only) the sum of
> Â front capacity and back capacity. I can't find any sensible use of
> this info, and, besides,
> Â the careless reader could assume capacity() works as
> std::vector::capacity(), which
> Â raises usability concerns. My suggestion is to remove this. FWIW, we
> had a similar
> Â discussion about capacity() in Boost.PolyCollection, and that member
> function finally
> Â was dropped.
The capacity from Boost.PolyCollection was quite another beast. Here
capacity is just like vector's capacity(), namely the sice of the
contiguous chunk of memory.
> Â - front_free_capacity/back_free_capacity could be more aptly named
> Â front_capacity/back_capacity.
But unlike capacity() in vector, these actually tells you how many
push_back/push_front you can do without reallocation. There is no
definition of "back capacity" because there is no "middle".
>  - reserve as an alias to reserve_back has usability problems as
> discussed with capacity.
Again, what about generic code?
> Â I'd remove them.
> Â - For symmetry, shrink_to_fit could be complemented with
> [front|back]_shrink_to_fit
Is back_shrink_to_fit then an alias for shink_to_fit or does it actually
consider both ends?
> be documented officially. I'm assuming that erasure behaves analogously
> (i.e. space
> is given back to the side that's closer to the erasure point) --in
> particular, inserting
> an element and erasing it immediately after should return front and back
> capacities
> to their original values.
Anything else would be very inefficient.
>
> REVIEW FOR BATCH_DEQUE
>
> d.segment_[begin|end](), which allows us to use a range for in:
>
> Â for(auto segment:d.segments){
> Â Â Â for(auto& element:segment){
> Â Â Â Â Â // do something with element
> Â Â Â }
> Â }
This is a good idea IMO.
> 2. As with devector, why is serialization code is so complicated and
> does not
> simply rely on
> boost::serialization::stl::collection_load_impl|save_collection?
Let's wait for the performance test.
> 3. Tests look good.
>
> 4. Postconditions on front and back capacity are not documented.
You mean front_free_capacity etc? What condition did you have in mind?
The funcions are const.
kind regards
Thorsten