Re: [boost] [metaparse] Update review

Subject: Re: [boost] [metaparse] Update review
From: Abel Sinkovics (abel_at_[hidden])
Date: 2015-06-04 01:30:28

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_code/main.cpp) 
"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