Re: [pfr] Ready for informal mini review

From: Antony Polukhin (antoshkka_at_[hidden])
Date: 2020-10-20 17:15:01

пн, 19 окт. 2020 г. в 23:44, Andrzej Krzemienski <akrzemi1_at_[hidden]>:
>
>
>
> pt., 16 paź 2020 o 12:23 Antony Polukhin via Boost <boost_at_[hidden]> napisał(a):
>>
>> Hi,
>>
>> Issues noted during the review period were fixed. Mainly:
>> * global_ops.hpp is removed
>> * boost::pfr::ops::operator* were changed to functions and moved to
>> boost::pfr:: namespace
>> * Flat reflection was purged
>> * Big rewrite of docs
>>
>> I'd appreciate a look at the updated library
>> https://github.com/boostorg/pfr and its docs
>> https://apolukhin.github.io/magic_get/index.html
>>
>> Any comments and feedback is welcomed!
>
>
> Thank you for doing the mini-review.
> The documentation looks much cleaner, and it is much more intuitive.
>
> I have some minor observations for the docs.
>
> 1. Now that the Flat part is gone, the name of the library PFR becomes hard to understand. Unless you can retrofit some title to match the acronym (like, Position-based Field Representation) maybe changing the name would be appropriate?

I'd like to leave PFR abbreviation, but the title is a subject to
change. We have to find out a good one:
* Position-based Field Representation
* Pack of Fields Representation
* Pleasant Fields Representation
* Primitive Fields Representation
* ???

Your title seems to be the best so far.

> 2. The first example should be as short as possible, to convey the feel of the library within as few seconds as possible. Reading command line args and opening a file goes against this goal. Maybe:

Good point! Fixed

> 3. Section "Usecase example"  can be moved to a separate page with motivating examples. Especially that you may wish to expand it. The current comparison may not be fair. Without Boost.FPR what people do is not to put all aggregate members to function `db::insert`, but rather to use a macro or a specialization of `std::tuple_get` to enable index-based access to a type. So a fair comparison would be one where `std::tuple_size` or `BOOST_FUSION_ADAPT_STRUCT` is mentioned.

I've added a comparison with hand-written customizations (those are
simpler and shorter than BOOST_FUSION_ADAPT_STRUCT usages)

I'd rather leave the section where it is, because I do not plan to
extend it. All the other samples will go to the tutorial section.

> 4. "simple aggregate" should not be listed as a "limitation, but instead be defined as a concept or type requirement. Definitions of functions in PFR should refer to it as requirements.

I'm not very comfortable with the concept changing its requirements
depending on the C++ standard and compiler implementation.
'Limitation' seems to be a better word.

> Regards,
> &rzej;

Fixed docs: https://apolukhin.github.io/magic_get/index.html

-- 
Best regards,
Antony Polukhin