Re: [boost] [function types] Formal Review begins today.

From: Jody Hagins (jody-boost-011304_at_[hidden])
Date: 2005-06-06 10:08:16

First, this looks very cool, and I think it will get a lot of use even
from those who are a bit scared of full MPL.

This is not my official review, but simply some questions/comments about
the documentation, specifically the things I think of as I read it for
the first time...

I am not a specialist in grammar, and I have the distinct disadvantage
of American English as my native language (well, most will say that
South Carolina-ese is English).  However, I had to read the first
sentence of the MOTIVATION section three times to decipher its meaning. 
I think it could be better understood if it were restructured...

Also, in the first paragraph of MOTIVATION, the author asserts "common
applications" and "well known special cases" of which a large number of
people are not familiar.  IMO, if you reference a motivating case, you
should explain it a bit more, or provide references to examples.

I am similarly confused by the last paragraph of MOTIVATION.  What is
meant by "It covers the functionality of the function_traits class?"

After reading the OVERVIEW and MOTIVATION, I am still at a loss for what
benefit this library will provide.  I can make some reasonable
assumptions, but I would think that I should have a much better picture
than I currently have from reading those two sections.

Obviously, I am either a total idiot, or I am just having a bad day
(please do not confirm the former if you know me personally).  I am
having a hard time groking the docs for CLASSIFICATION, especially the
summary.  I do not think I should have to read the full description to
get an idea of what the function does, yet after reading the summary, I
thought, "Hmmm... maybe a read through the details will help."  To me,
this indicates a little more information should be in the summary.

Also, the CLASSIFCATION docs say that it tests to see if a type is an
element of a set of other types, and gives several .cpp files as
examples.  However, after reading the entire specification for
is_function_type, I have these questions...

How do I build a set of types?  How do I test for a type in a set?  All
examples show one type, not a set of types.  

What should happen if I put types in the set that are not function
types?  Should I expect a compile error, or is it up to the programmer
to know specifically that the types belong in that set?

For the function calls, the descriptions make some assumptions (like, it
must be called with a function type).  Are these suggestions for good
style, or will the implementation provide a compile-time assertion, or
will you ger undefined behavior if you violate the assumption?

function_type_arity: Should probably put note about BOOST_FT_MAX_ARITY
here (or reference the CONFIGURATION section) to make it obvious early
about the max arity.  What happens if you call it with a non-function? 
Do we get a compile time assertion?

function_type_parameter: should probably say that the index starts at 0
(this is the common C/C++ method, but stating it explicitly would be
helpful).  It would also be helpful to be a bit more explicit about the
difference between function_type_parameter and
function_type_parameter_c, since not everyone groks MPL as second
nature.

How do functors fit in here?  If a class overloads operator(), is it
considered a function type?

In the ENCAPSULATION section, I think the author is again making too
many assumptions about the detailed knowledge of MPL.  This library,
while using the MPL, and imitating some aspects, does not require full
knowledge of the MPL.  However, the docs seem to take too much for
granted, and make too many assumptions.  I'd like to see more detail,
especially in the descriptions.

After my initial reading, I am still not entirely sure about
function_type_signature.  I would like to see more description
information.  Also, I do not like the "types" member.  What is the
rationale for the ordering of the types (return, [class], parameters)? 
Callers would have to query even more information to determine how to
interpret types.

Also, if you remove c-v qualifiers, how can a caller ever get what was
truly passed to function_type_signature?  Maybe adding something like
raw_representee, or something else so that the caller can see the true
T?

function_type: looks very intetresting.  Unfortunately, none of the
thoughts/questions in my head about usability are answered by the
examples.

PORTABILITY: If you have not tested with BCB, maybe you should (or get
someone to do it).  Or, at least note that it is most likely broken on
that compiler, w.r.t. function references.  Specifically, BCB6 does not
properly distinghush...

int (func)()
int (*ptr_to_func)()
int (&ref_to_func)()

Should the CONFIGURATION section go before the detailed descriptions of
the function calls?

In general, I do not like using test assertions as examples.  I know
lots of boost stuff uses this methodology, but it does not provide much
information.  If tests are used, then they should include LOTS of
documentation describing exactly what is (and is not) happening, and
sufficient justifications.

After reading the docs, I think many will be at a loss for concrete
examples of real-world use for this library.  Again, in a library like
this, the documentation should include LOTS more examples, and each
example should be fully documented.  Otherwise, the curve to usage is
enormous.

Finally, I'd like to see some examples that support the motivation.  The
author cites some reasons for the library to exist.  I want proof, in
the form of examples that show how difficult (or impossible) it is to do
what he wants without the library, and then examples showing how the
same problem is easily solved with the library.  I do not think this is
just unique to this library.  I think it is important for all libs,
hint... hint...

I am under great time constraints (work, helping coach my son's baseball
team, travelling every Fri-Sun with my daughter's softball team, etc.),
but if I can find time to actually use the library, I'll issue a review
as well.

Thanks for the submission!