
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