[boost] [type_erasure] Review

Subject: [boost] [type_erasure] Review
From: Karsten Ahnert (karsten.ahnert_at_[hidden])
Date: 2012-07-26 15:20:59

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