Re: [boost] Re: [Formal review] Collection Traits library

From: Rob Stewart (stewart_at_[hidden])
Date: 2004-05-05 11:44:12

From: "Thorsten Ottosen" <nesotto_at_[hidden]>
> 
> | I'd prefer "primary specialization" to "default" in the comments.
> 
> ok. Is that the correct term?

A "specialization" of a template is the result of instantiating
it, so the more accurate term, as given in Vandevoorde and
Josuttis, is "primary template."

> | Lines longer than 80 columns in a number of files.
> | detail/common.hpp is particularly hard to view in 80 columns.
> 
> is that a requirement? If so, I will change it.

It's in http://www.boost.org/more/lib_guide.htm#Guidelines.

> | Why isn't result_iterator_of implemented in terms of iterator_of
> | and const_iterator_of?  As it stands, it duplicates the code in
> | iterator.hpp and const_iterator.hpp.  Collocating their
> | implementation would reduce the chances of maintenance errors.
> 
> True. But the implementation is so simple, that it hardly helps
> to put the code thruogh another layer of templates. Doing so might
> give longer compiles. And some people care about minimal headers.

I should think the minor increase in compilation time this would
trigger would be largely unnoticed and the reuse and resultant
prevention of future errors would be wise.  You might think that
the tests will discover any such problem, but only if the
maintainer thinks to change all affected tests.  Through reuse,
any correction made to, say, iterator_of will be a correction to
result_iterator_of.

> | Couldn't the primary specialization of size_type be to use SFINAE
> | to check for a nested size_type type or else std::size_t?  You
> | wouldn't need any specializations then.
> 
> I guess it could. My only problem is that I don't know how portable
> this is. AFAIK, detecting nested typedefs only works with Comeau.

I don't know the answer.

> | "Sise" [sic] is misspelled in sizer.hpp.  Shouldn't sizer.hpp be
> | in detail?
> 
> yes. maybe. I don't find it very useful. But if people do,  I can put
> BOOST_COMPILE_TIME_ARRAY_SIZE() macro in detail/array_size.hpp.

Right now, all code in sizer.hpp is in
boost::collection_traits_detail; hence my suggestion.

> | The Introduction is missing a motivation section.  It goes from a
> | short description to an example.  It should help the reader
> | understand why the library is valuable.
> 
> IMO the introduction is the motivation. what would you like to see in a motivation section?

"When writing generic code that works with Standard Library
containers, one often finds it desirable to extend that code to
work with other types that offer enough functionality to satisfy
the needs of the generic code, but in an altered form.  For
example, raw arrays are often suitable for use with generic code
that works with containers, provided a suitable adapter is used.
Likewise, null terminated strings can be treated as containers of
characters, if suitably adapted.  This library provides the means
to adapt Standard Library containers, null terminated strings,
std::pairs of iterators, and raw arrays, such that the same
generic code can work with them all."

> | The Introduction is missing discussion of the use of namespace
> | scope functions to do what heretofore would be done via member
> | functions.  Instead, the example and the sentence immediately
> | following it imply this change of syntax.
> 
> Wouldn't it only duplicate stuff in the two links to CollectionCocept and ExternalConcepts?

In a sense, yes, but what I'm proposing is a much abbreviated
form:

"To allow generic code to work with objects that conform to the
ExternalCollectionConcept, this library provides a set of
namespace scope functions and <A
href="http://www.boost.org/more/generic_programming.html#type_generator">type
generators</A> that provide a standard interface for the required
functionality.  Whereas one might write, "c.end()," to get the end
iterator from a Standard Library container, c, using this
library, it would be written, "boost::end(c)."  This change in
syntax is necessary to make the same interface possible with the
other supported types such as null terminated strings."

-- 
Rob Stewart                           stewart_at_[hidden]
Software Engineer                     http://www.sig.com
Susquehanna International Group, LLP  using std::disclaimer;