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