On 07/26/2012 11:22 PM, Steven Watanabe wrote:
AMDG
On 07/26/2012 12:20 PM, Karsten Ahnert wrote:
Hi,
here is my review for Steven Watanabe's TypeErase library.
First of all, I want to thank Steven for developing this library. My overall evaluation is that TypeErasue should be accepted as boost library. I did not looked very deeply into all details or the implementation but I tried to solve some problems with the library. So, I only looked at the library from the users perspective.
= 1. What is your evaluation of the design? =
The design looks clear and concise. I have only two small point:
* Why do I need to specify relaxed_match to get a default constructable any?
Except for the absolutely essential components, (e.g. binding_of, concept_of, placeholder_of, any -> any conversions), all behavior of an any must be specified explicitly. I bundled a lot of functionality into relaxed match. (the default constructor, assignment in terms of copy_constructible). I could split it up, if it's needed, but I assumed that all these things that make any act more like a normal object are likely to be used together.
In my opinion any should be default constructible by default. I think this is what most user expect, and std::function, and boost::any are also default constructible. But of course, this is up to you.
Maybe a empty() function would be nice to check if any holds a value (in case it is default constructable).
Okay.
I think a hint about a default constructable any should be in the tutorial. It is important for users.
* Sometimes it is not clear, why some constructors work and others not, at least for me. Details are shown below.
= 2. What is your evaluation of the implementation? =
I did not looked into the implementation.
= 3. What is your evaluation of the documentation? =
* The documentation looks good. But I think more real-world examples would be very useful. I also encountered some problems where I could not find a hint how to solve it. For example, consider the case where you need an any (for example in a recursive data structure) to be a member of some other class:
struct my_class {
private: any_type a; };
What kind of constructor do I need to set up this any properly? I came up to define two constructors:
struct my_class { template< class T > my_class( T t ) : a( std::move( t ) ) { } my_class( any_type a_ ) : a( a_ ) { } private: any_type a; };
I'm not sure why you need this. Either constructor should be sufficient.
The second one alone is not sufficient. But the first one alone is ok. At least this what the compilers tells me if I try it with some combinations of any.
template< class Context > class not_ : struct unary_expr< bool , Context > { template< class T > not_( T child ) : unary_expr< bool , Context >( child ) { } bool operator()( Context c ) { return ! this->m_child( c ); } };
I think this is ill-formed with Concept = void.
Yes, I have a specialization for void.
This works quite nice, but I always had problems to figure out how the any type are constructed, for example
using expr_type = boolean_expr< void >; using not_ = not_< void >; using and_ = and_< void >; using or_ = or_< void >;
true_ t; false_ f;
// why does this not work? // expr_type e3( not_( f ) ); // cout << e3() << endl;
// why does this work? expr_type e4( and_( t , or_( f , t ) ) ); cout << e4() << endl;
The first example expr_type e3( not_( f ) ) does not compile. It fails because not_( f ) is interpreted as a function pointer:
e3 is a function declaration. It's equivalent to: expr_type e3(not_ f);
ok.