Boost logo

Boost :

From: Eric Friedman (ebf_at_[hidden])
Date: 2006-05-09 21:01:53

My review of the proposed Boost.Fusion library follows. In general, I
think the library will be useful, at least for library developers. My
time for the review was very limited, and so I have not actually spent
any significant amount of time using the library. Hopefully my feedback
will be useful nonetheless.

> - What is your evaluation of the design?

I appreciate the concern about efficiency and the consequent use of
laziness in constructing views. But since the underlying sequence is
held by reference, a view cannot outlive the underlying sequence. What
is the solution if this is in fact desired? Is the solution to use
as_vector or one of its kin on the resulting lazy view? There is no
discussion or acknowledgment of this in the documentation as far as I saw.

The extensibility feature of the library is potentially very useful.
That several foreign libraries -- MPL, std::pair, etc. -- are supported
as first-class Fusion sequences without modification to those libraries
is compelling. From my brief glance at the implementation, the
extensibility feature also seems to simplify the support of the several
Fusion sequence types. However, it seems that there is significant
overhead in taking advantage of Fusion's extensibility. For major
existing components such as std::pair, this overhead can be taken care
of once and included with Fusion. But for an end-user of the library the
story is far more complicated. In particular, the amount of effort
required to support even the trivial example_struct indicates to me that
very few user types will ever have Fusion support in this manner. Dave
Abrahams made a similar comment, but I think this more thought is needed
here. If I had more time for this review, I would have done some of the
thinking myself :)

With regard to naming, I think fusion::pair is a bad name. Perhaps it
should be instead named fusion::keyed? I think the name pair should be
reserved for two values (std::pair) or two types (mpl::pair). The mixing
of the two was unexpected.

I find the metafunctions in the library not as straightforward as they
should be. On the one hand, it seems nearly all of the metafunctions are
the same in functionality and use, simply providing the result type of
the various functions in the library. Then there is something like
fusion::result_of::value_of that does not correspond to any function.
Why? Of what function exactly is this the result? I think metafunctions
like this should be in the plain fusion namespace.

Aren't the arguments to the function object called by the fold algorithm
"backwards?" That is, to follow the MPL convention, it would be (state,
elem) not (elem, state). Maybe other functional languages do it
differently? If so, I am not sure where it is better to have
consistency, though I would lean towards consistency with MPL.

I was going to suggest an "apply" algorithm, which would invoke a
function with the elements of a sequence as arguments. From reading the
newsgroup, I see this comes in the form of unpack_args. As noted
elsewhere, this should be documented.

> - What is your evaluation of the implementation?

I did not look at it extensively.

> - What is your evaluation of the documentation?

There needs to be more discussion of where Fusion is appropriate or not
appropriate. I see there has been discussion about this on the newsgroup
recently. This sort of discussion should appear in the documentation as
well. Thinking about how to explain this may also lead to a good simple
example to demonstrate a case where the library would be useful.

For instance, I initially wondered why there is not a filter algorithm
that takes a runtime predicate. The answer, of course, is that this
would not be possible, since the length of the resultant sequence would
not be known at compile-time. Though this is obvious upon even the most
basic reflection, it was not immediately obvious to me. Documentation
could have helped here.

There should be discussion of the approach Fusion takes with respect to
laziness and holding the underlying views by reference. The performance
claim made here is probably, though maybe not, obvious. More important,
though, and also not discussed is how to make a deep copy of a view, in
case it needs to outlive its underlying sequence.

Not to unnecessarily belabor the point, but the documentation badly
needs examples other than how to extend the library. I understand the
problem of coming up with a simple (but also non-trivial) example is not
easy. Even if the example is somewhat involved or complex, ddiscussing
it in the documentation would be incredibly useful to people such as
myself approaching the library for the first time. As for the audience
of such a discussion, I think you can assume your readers are familiar
with metaprogramming and other advanced C++ techniques. What I want is a
discussion that shows me how Fusion can be so useful -- not merely
leaves me with the sense that it probably is, kinda, maybe :)

I think the documentation as a whole needs some reorganization. If
nothing else, I think the documentation for the metafunctions should
appear in a separate section from the regular functions. As it is, the
metafunctions appear quite predictable, mostly for determining return
types, and so the metafunction reference disrupts the flow of reading
the documentation. On a related note, if there is in fact anything
unusual or special about some of the metafunctions (apart from my
comment above about value_of, etc.), I missed it in the monotony of
their documentation.

I also have a few minor complaints about typos, etc. in the
documentation, which I note below.

> - What is your evaluation of the potential usefulness of the library?

Since my time with the library was limited, I am left only with an
impression. My impression is that it is potentially very useful in the
development of libraries.

> - Are you knowledgeable about the problem domain?

Somewhat. I used MPL quite a bit in the development of Boost.Variant,
and I have used metaprogramming techniques in other projects, for
serialization/deserialization of data to pre-established wire formats.
Particularly in the latter project, I think Fusion's facilities would
have been helpful.

Minor documentation issues:
- example for zip algorithm has syntax "make_vector((1,2),('a','b'))" -- 
not real, is it?
- example for pop_back spells it "___pop_back___" for some reason

Boost list run by bdawes at, gregod at, cpdaniel at, john at