[boost] Re: review request: container_traits

From: David Abrahams (dave_at_[hidden])
Date: 2003-10-04 14:47:03

Pavol Droba <droba_at_[hidden]> writes:

> On Sat, Oct 04, 2003 at 12:24:19PM -0400, David Abrahams wrote:
>
>> I haven't looked at the implementation of container_traits, so I'm not
>> sure how to address that, but this is not a complicated idea.  Would
>> you design functions this way?  
>> 
>>   template <class T>
>>   boost::tuple<A,B,C> get_all(T);  // common core
>> 
>>   template <class T>
>>   A get_a(T x) { return get<0>(get_all(x)); }
>> 
>>   template <class T>
>>   B get_b(T x) { return get<1>(get_all(x)); }
>> 
>>   template <class T>
>>   C get_c(T x) { return get<2>(get_all(x)); }
>> 
>> Only under extraordinary circumstances, IMO.  Maybe you have such
>> circumstances here, but that would be pretty extraordinary ;^)
>>
>
> Your example is very illustrating :)
>
> Please have a look to container_traits implementation. 

OK.

I note the illegal use of a name reserved to the implementation:
__function_type

What I see there makes a perfect case for what I've been talking
about: each of these container_traits<T> instantiations contains a lot
of declarations and I can easily imagine only wanting one or two.  I
see no reason at all to have begin et. al as member functions instead
of using separate overloaded free functions.  And even if you don't
buy the efficiency argument (you should), reserving an
all-encompassing name like container_traits<T> is a bad idea if it
means the component needs to be modified each time someone wants to
add new traits (and I can think of many we might want to add
e.g. insert_invalidates_iterators).

While we're discussing efficiency, I'd also like to see the
implementation use partial specialization instead of a big apply_if
switch to select the implementation when available.

> A good example is a specialization for build-in array ( like char[]
> ).  Chalenge there is to select base type and size of the
> array. Once this is performed, all traits are calculated with ease.

"With ease" doesn't neccessarily mean "for free".

> This specialization has the exactly the same structure as the
> example above.
>
> For the container_traits the harders part is to resolve the
> type. Once this is done, single traits are just a matter of simple
> forwards.  So I don't really see the way how the separation of
> individual traits would gain some performance.

Each nested typedef and member function signature costs resources in
the compiler when the template is instantiated.  Now, whether one
class template instantiation and N "trivial" nested typedefs (where N
is the total used in your composite traits class) cost more or less
than M class template instantiations, each with a trivial nested
typedef (where M is the number of individual traits an average program
is interested in), I can't say.  In the worst case, M is much less
than N, and grouping traits doesn't pay off.  To me it looks as
though that will be the case often enough to warrant breaking the
traits up.  That becomes increasingly true as we find new properties
of containers that we want to represent.

>> > There is another reason, why keep the interface in current state.
>> > container_traits follow the same purpose as iterator_traits for the
>> > context of containers. It makes sense to keep them in sync. If there
>> > were reasons to define iterator_traits in the way the are, the same
>> > reasons apply for container traits.
>> 
>> But there *weren't* reasons to define iterator_traits that way; nobody
>> knew any better and until the Boost type traits library came along and
>> I insisted on separate traits classes, degenerate multi-valued traits
>> were the norm.
>
> I understand, but there is slight distintion between traits from
> Boost.TypeTrais library and those declared in iterator_traits and
> container_traits.
>
> You can see the later two as a description of a given entity. 

I see no distinction whatsoever in that respect.  

> All elemnts together are related to one entity and they are
> dependant on each other. 

It is not possible to have any dependency cycles in the nested
typedefs, so they're not all dependent on one another.  Just
refactor.  This is a simple principle.

> Most of the traits in TypeTraits library are unrealted to each
> other.

I don't think so.  Have you looked at the implementation of is_enum
recently?  There is an elaborate interdependency DAG between is_enum
and is_class and a number of the other traits on many platforms.

> It could be error prone to define for instance value_type and
> reference_type of the iterator in two separate traits, because they
> can easily get out of sync.  

They could, but hardly "easily", because the traits would probably be
written right next to one another and especially not easily when the
default implementation conveniently just reaches inside the iterator
itself, where usually all the typedefs are grouped.

> In a single trait class, all traits can share the common parts. This
> could also simplify writting of such a class.

Sorry, I find that argument unconvincing.  You can easily factor out
common parts into separate metafunctions.  I'm going to stop arguing
about this now; if I haven't convinced _you_ by now, I doubt anything
will.  Let me just close by saying that "decoupled is better than
coupled" is a well established principle, and its sad that we (me
included) seem to have to keep relearning that lesson by making the
same mistakes.  I'm just thankful Fredrik spotted this instance of the
mistake, and I hope you and Thorsten will respond accordingly.

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com