Re: [boost] [review] The review of Boost.DoubleEnded starts today: September 21 - September 30

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