Re: [boost] [review] a mini-review of containers

Subject: Re: [boost] [review] a mini-review of containers
From: Ion Gaztañaga (igaztanaga_at_[hidden])
Date: 2011-08-12 16:51:42

El 12/08/2011 14:07, Thorsten Ottosen escribió:
> Hi Ion & John,
>
> I apologize for not having time to do a full review. I base these
> comments mainly on the documentation.

Thanks for your time Thorsten.

> C. slist should probably be called forward_list like in the new
> standard, and should probably be interface compatible with the standard
> design.

My idea is to add another container with forward_list guarantees and 
standard interface. slist would be there for backwards compatibility 
(with constant-time size()).

> D. I skimmed some of the unit-tests. I saw that some sections are
> commented out. Maybe a little more testing could be done.

Ok.

> E. I didn't look at the map/set implementation; do you use the same
> optimization as Boost.MultiIndex that saves the color in the parent
> pointer (as a bit).

Yes, it's based on Boost.Intrusive rbtree container and that 
optimization is activated.

> F. I noticed that you use std::reverse_iterator. Why not use
> boost::reverse_iterator? IIRC, it has some minor benefits.

Ok, I will look at it.

> G. flat_xxx. (a) We already discussed the issue of exposing the
> underlyng container. I think that would be most natural, and provides
> use with an escape hatch when they need it.

Right, would this be acceptable:

flat_xxx flat;

//Move returned
flat_xxx::impl_t i(flat.extract_implementation());

//Modify
i.insert()
std::sort(i.begin(), i.end());

//Integrate it again, checks in
//debug mode
flat.inject_implementation(::boost::move(i))

> (b) I think all these containers should allow one to decide the wrapped
> sequence. I have several types of sequences I would like
> to be able to use. Sure it takes some work to handle alocators and such,
> but it makes these much more useful IMO.

Yes, we should decide the underlying interface, say std:vector C++11.

> (c) At some point, long time ago, I suggested that one should be able to
> construct a set/map with an already sorted sequence. This was based on
> actual use cases; one sometimes have access to an already sorted
> sequence. I think the most elegant way to support this
> would be to make a new constructor that forwards; that is, use the same
> approach as boost::optional such that we can say

flat_xxx containers have those functions:

flat_set(ordered_unique_range_t, InputIterator first, InputIterator 
last, , const Pred& comp = Pred(), const allocator_type& a = 
allocator_type())

flat_multiset(ordered_range_t, InputIterator first, InputIterator last, 
const Pred& comp = Pred(), const allocator_type& a = allocator_type())

I don't know if that would be enough for your use cases.

Best,

Ion