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? Maybe a empty() function would be nice to check if any holds a value (in case it is default constructable). 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; }; = 4. What is your evaluation of the potential usefulness of the library? = The library is very useful and can be applied to a large number of problems. The library provides an alternative to classical OOP and can be used in many situations. = 5. Did you try to use the library? With what compiler? Did you have any problems? = I tried with gcc-4.7 and gcc-4.6 without problems. I wrote a test application with some kind of an expression system which can be build at runtime. (Maybe this example would be useful for the docs?) My main any definition is template< class Return , class Context > struct requirements : public mpl::vector< te::copy_constructible<> , te::typeid_<> , te::callable< Return( Context ) > , te::relaxed_match > { }; // ... template< class Return , class Context > using expr = te::any< requirements< Return , Context > > ; template< class Context > using boolean_expr = expr< bool , Context >; Then you can build base classes for unary and binary expressions which model the expr concept and contain the subexpression, for example: template< class Return , class Context > class unary_expr { public: typedef expr< Return , Context > expr_type; template< class T > unary_expr( T child ) : m_child( std::move( child ) ) { } unary_expr( expr_type child ) : m_child( child ) { } expr_type& child( void ) { return m_child; } const expr_type& child( void ) const { return m_child; } protected: expr_type m_child; }; At this point I needed some time to figure out that you need at least these two overloads of the constructor. Actually I am not sure if these two overloads are sufficient for all use cases. Some clarification in the docs or any hints would be very useful here. Having the base classes it is very easy to implement concrete expressions, for example: // Boolean expressions struct true_ { template< class Context > bool operator()( Context ) const { return true; } }; struct false_ { template< class Context > bool operator()( Context ) const { return false; } }; 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 ); } }; 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: rule_engine.cpp:35:16: error: too few arguments to function ‘expr_type e3(not_)’ rule_engine.cpp:34:15: note: declared here I have no idea why the second one compiles. = 6. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? = I spend about 10 hours with studying the library. = 7. Are you knowledgeable about the problem domain? = Some experience in boost.any, boost.function and a self written type erasure application. Not too much. Thank you again! Karsten
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.
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.
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.
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);
rule_engine.cpp:35:16: error: too few arguments to function ‘expr_type e3(not_)’ rule_engine.cpp:34:15: note: declared here
I have no idea why the second one compiles.
e4 can't be parsed as a function declaration. In Christ, Steven Watanabe
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.
participants (2)
-
Karsten Ahnert -
Steven Watanabe