Hi Edward, Thank you for the updated review. On 2015-06-04 06:45, Edward Diener wrote:
- What is your evaluation of the design?
A couple of quibbles about the design based merely on my own sense of 'naming'. Since I have roundly criticized "naming wars" <g> in the past I no doubt leave myself open for criticism, but these are just personal suggestions, which I can live without:
* BOOST_STRING is very generic and can easily lead to a macro clash. Something like BOOST_METAPARSE_STRING or maybe BOOST_MPS_STRING would be more distinct and safer. I favor shortened names for libraries in macros to be 3 characters rather than 2 characters so as to decrease macro clashes. I'm sure it won't remain BOOST_STRING. I'm open to BOOST_MPS_STRING as well.
* The various 'foldxxx' names, even with the Cheat-Sheet explanation of their name formation in the docs, seems needlessly confusing. Would it not be better to make longer, less confusing names, which actually spell out what the metafunctions are about. Is it really so hard to write "fold_left', 'fold_right', 'fold_left_fail' etc. etc. Sometimes clarity trumps how many keys one types. I see your point. foldl and foldr are common in functional languages, but the rest of the letters (p, f) are just made up. I'm not sure though how to choose (long) names that help understanding what the function does without checking the documentation (eg. fold_left_fail wouldn't tell me what fail is there for without checking it). Names like
fold_right_with_initial_value_from_parser (instead of foldrp) fold_left_with_initial_value_from_parser_and_fail_if_parser_partially_applies_at_end (instead of foldlfp) are useful, but horribly long. I'm open to suggestions.
* A general discussion about the purpose of the functionality of the library would be welcome. Such as the possible metaprogramming scenarios in which a programmer would want to accept and analyze ( parse ) an MPL string in creating his metafunctions. It sounds like this could be combined with a few (easy to understand) examples on what the library does (as pointed out earlier by other reviewers) and be part of the documentation as the guide to help quickly evaluate if one needs it or not.
* A general comparison of when it might be better/worse to use Metaparse as oppose to Boost.Spirit. Pluses and minuses of both approaches might be discussed. Will be added.
* On the first page of documentation it mentions the various types of output which Metaparse can generate. Both 'types' and 'metafunctions' are very understandable, but whatever is meant by "Objects" and "Callable C++ functions" doesn't follow the definition of these terms in C++ which I know. Some discussion, or link, of what these two types of output mean in the context of Metaparse should be given. object: a runtime object, the type and constructor arguments of which are determined by the metaprogram (the EDSL parsed with Metaparse). For example the parsers of the regex example (https://github.com/sabel83/metaparse/blob/master/example/regexp/main.cpp) "returning" sregex objects.
callable c++ function: a callable object constructed by a metafunction. For example the parsers of the compile_to_native_code example (https://github.com/sabel83/metaparse/blob/master/example/compile_to_native_c...) "return" objects that can be (and in the example are) called at runtime. So the parser is generating runtime code. These can be added to the documentation.
* The Versioning section appears to be wrong in quite a number of places. Please proofread what was written there and correct/update it. Thank you for pointing it out. I'll fix them.
* In the Reference section, among the items enumerated at the end are 'Metafunctions and metafunction classes' and "Utilities' but when you click on the links to go the actual headings it looks like the metafunctions are split before and after the utilities. Thank you for pointing it out. I'll fix this as well.
Regards, Ábel