Re: [boost] [type_erasure] Review started (July 18-27, 2012)

Subject: Re: [boost] [type_erasure] Review started (July 18-27, 2012)
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2012-07-27 23:43:34

AMDG

On 07/27/2012 12:46 PM, Eric Niebler wrote:
> On 7/17/2012 10:13 PM, Lorenzo Caminiti wrote:
>> Please state clearly whether you think this library should be accepted
>> as a Boost library.
> 
> Yep.
> 
>> Other questions you may want to consider:
>> 1. What is your evaluation of the design?
> 
> Solid. One question:
> 
> Does concept_interface really need to explicitly inherit from Base? What
> about mpl::inherit_linearly? I suppose this has problems with
> overloading, right? Blarg. The overloading thing sure is nasty, but I
> have no suggestions. :-(
> 

A straight linear hierarchy also allows members
to be hidden.  e.g. the specialization for
bidirectional_iterator overrides the iterator_category
typedef from forward_iterator.  Also, I'd still
need a parameter, since Base doubles for CRTP.

>> 2. What is your evaluation of the implementation?
> 
> I didn't look.
> 
>> 3. What is your evaluation of the documentation?
> 
> The reference docs are thorough. The user docs could be more gentle. I'm
> afraid it will be rather imposing for most non-guru C++ programmers.
> 
> There is very little motivation given. Check out Sean Parent's BoostCon
> 2012 talk on value semantics[^1] to see a really good example of
> building motivation using a concrete example, and then presenting the
> solution. It helps get the points across. The one and only complete
> example in the docs (polymorphic range formatter) has this only for
> building motivation:
> 
>>  This example demonstrates using Boost.TypeErasure to implement a virtual "template" function. 
> 
> That's a pretty thin motivation. WHY is a virtual template interesting?
> What problem does it solve? Is this really all that you have to say
> about this example?
> 

Noted.

> In short, I'd put a handful of examples at the front of the users' guide
> that are relate-able and illustrative. Otherwise, you just have a
> listing of features and sample usages, with no motivation, no rationale,
> and no thread tying the features together.
> 

Okay.

> It might even help to describe what "type erasure" is, how it's been
> traditionally used, and what one of the simpler examples would look like
> without Boost.TypeErasure. That way, folks can get a feel for the
> drudge-work that your library saves them.
> 
> Aside: I don't much care for the name "TypeErasure". In purpose, it's
> much like the proposed Boost.Interface library. And that is closer to
> how the library is used: to define an interface and build a type that
> implements it. The fact that it uses a mechanism called "type erasure"
> to achieve that is really just an implementation detail. Some might feel
> the word "interface" comes with too much OO baggage, or imply reference
> semantics. I don't feel that way, but then you might consider
> "StaticInterface" or "ValueInterface". Actually, I quite like the latter.
> 

I'm open to suggestions, but I'm not sure what
the "Static" in StaticInterface refers to, and
I'm also not convinced that ValueInterface really
appropriate, since the library supports references,
not just value semantics.

> I'd also consider going through the docs and changing references to
> "requirements" to "interface".
> 
> Details:
> 
> On "Using references"
>> Note
>>
>> _self is the default placeholder, so it is easiest to use _self&. We could use another placeholder instead. any<__typeid<_a>, _a&> has exactly the same behavior
> 
> __typeid should be typeid_
> 

Fixed.

> "Concept Maps"
> A link here to your built-in Concept Definitions would be nice. "See
> _here_ for a comprehensive list of built-in concept maps that can be
> specialized like this."
> 

Not all built-in concepts can be specialized.
I suppose I should indicate which ones can be.

> "Associated Types"
>> Referring to the full name of the associated type can be cumbersome when it's used many times. 
> 
> This, after a line of code that reads "any_cast<int>(*x)". After some
> head-scratching, I'm inferring that *x returns an any. Of course. And
> same_type is a promise that deduced<pointee<T> > will always come out to
> be "int", so please just hard-code that in the interface and run
> whatever checks are necessary to guarantee that that is true. This is
> all clear after reflection, but it would be nice if the docs were more
> explicit on this point.
> 

Done.

In Christ,
Steven Watanabe