пн, 5 окт. 2020 г. в 03:30, Gavin Lambert via Boost
This is not (yet?) a review, although I guess this could be counted as a partial review towards the current state of the docs; but after reading them I have several questions.
1. Why is the term "reflection" used at all? As far as I am aware, this is primarily used to refer to accessing member names from the structure, which is not something that this library provides at all, so at best this seems highly misleading. The original "magic get" name seems more appropriate since it is primarily about extending tuple-get to basic structures without boilerplate macros.
Any name is bad: * magic get - gives no clue on what the library does * reflection - according to wiki "reflection is the ability of a process to examine the type or properties of an object, and modify its own structure and behavior.". PFR library gives you the ability to do that, but not on such a great level as some Java/C# users are used to. * "tuple representation" - that's quite good, but I'd like to give a hint to users, that the library does simple reflection Here comes the plan: * s/reflection/tuple representation/g * add a few sentences in the Motivation section and Readme to clarify that we are talking about very simple reflection cases
2. An up-front clarification on the limitations of supported structures would be nice. "Aggregate initialisable" is not a concept that everyone is familiar with.
I'll add an example.
3. What is the motivation for "flat" reflection to exist, at all? I can't find any explanation of why one might want to do it; other than completely disregarding type safety, which seems like a bad thing. (I assume there is some reason that I'm not aware of, but that's why an explanation would be nice.)
It's quite useful for some use cases (hashing).
4. Flat reflection is stated to be non-portable, raising further questions as to why it exists at all.
5. Many of the intro pages talk about "disabling loophole" with no explanation of what that is. The configuration macros page finally presents a link that doesn't really explain anything anyway, other than suggesting it is a Dark Magic that was intended to be banned but nobody had gotten around to it yet.
I'll try to improve the docs.
6. Speaking of the configuration macros page, it doesn't indicate what values are the defaults, other than it "auto-detects your compiler". I assume from the surrounding text that it would prefer to use C++17 and would use "loophole" (whatever that is) otherwise, but it would be good to make that (or whatever it actually does instead) explicit.
Defaults change depending on the compiler minor/major version, C++ standard version, standard library implementation and version. I'll add a short description on prefered implementations.
Granted #3 can't get you into *too* much trouble with the limitation on only supporting aggregate-initialised types... but on the other hand, type hierarchies are still significant for aggregates (it can be important to distinguish a "handle" from a plain int, or a Boost.Units value from another with different unit). And it feels like you're doing C++ wrong if you're using aggregate types much; they're only a little better than PODs.
(In all existing codebases I use, there are almost no aggregate types, although there are a few almost-aggregates that have simple initialising constructors, for example, or make member fields private and use a get-set method pattern "just in case". I imagine this is likely to be true of most real-world codebases.)
Yep, unfortunately C++ provokes you to write such code. That's a bad practice that could be avoided with PFR, see "Motivation and Examples" at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2141r0.html Probably I should add that motivation from the paper to the PFR docs.
Having said that, I can see some value in aggregate types as DTOs (for database/json/etc translation) and for reducing usage of std::pair and std::tuple, which is a good thing, though only if used in limited scope. But that usage doesn't explain "flat" either; the type hierarchy still should be important.
Precise reflection, on the other hand, seems more potentially useful, save for the unfortunate -- though understandable -- limitation on only aggregates. Having said that, I've personally never found a use-case for a tuple-like get interface for anything, so perhaps I'm just not the target audience for this library.
(I also have a strong dislike for aggregate initialisation being order-based in the first place; I would have preferred something like C99's named initialisation. C++20 is adding something that they're calling that, but is utterly useless and crippled instead of doing it properly.)
I'll try to update docs in a few hours to address the above issues -- Best regards, Antony Polukhin