Hi, On Fri, Apr 20, 2012 at 6:11 PM, Matus Chochlik <chochlik@gmail.com> wrote: [...]
Thanks in advance for any (negative or positive) feedback.
I am interested a lot in introspection in C++. I have been using another approach (and extended) boost.reflect. I'd like to participate in the effort. The document is rather lengthy though and I am not done reading (I've been away the past few days). Here are a few comments though: - Globally you start with the complex stuff. Yes what you did is very powerful and complete, but please consider the user point of view first. I am a user for reflection data and here is what I'd like to see when I am thinking about introspection in C++: #pragma reflect // generate metadata for the next language element sturct Reflected { int a; string b; double c; }; vector<Reflected> myData; CsvWriteHeader<Reflected>(myFile); for (auto& entry: myData) CsvWriteLine(myFile, entry); All the meta programming is very clever but has (IMHO) a smaller target audience, it's the mean, not the purpose. I believe you should start with the purpose then explain how you did it. So as a first piece of advice, I'd show how to reflect a POD (only data members) then a class (methods, static members) then a template and a template instance (if that is possible didn't check yet). - I think you are neglecting the aspect on how the user can choose the types that are going to be reflected. I believe not all types should have their metadata generated, at least to save on compile time on large projects. The way I did it in my preprocessor was using a pragma directive but it was much simpler than Mirror since it was only intended for classes. There migth be other ways, though, but it should be made more explicit. If reflection makes it into the standard, there shouldn't be any tools to generate the metadata besides the compiler. For the standard library, I guess that including a given header or using reflect operator would bring in the metadata. - I am not sure whether the "reflect" operator is a good idea. First I'd change the name to mirror since reflection is usually employed when the objects know their own metadata, which is not the case here. Then i guess that the operator should return a type, not a value, for easier use in metaprograms. #pragma mirror // (or reflect) struct S { … }; // defines a function that only accepted mirrored types template <class T> typename std::enable_if<mirror(T)::is_defined>::type f(T& t); If a new operator is too much, then a lot could be achieved using only template specialization, but that would not work with namespaces easily. ( mirror<T>::is_defined ...). It's probably Worth mentioning in a proposition. - I guess the "mp" namespace is something in the lines boost::mpl but there's no such thing in the standard yet. You should try to show use cases that are more accessible to main stream programmers and that show useful features without having to write and document a full page of code. Everything is still a bit messy, sorry if I said anything stupid, it's a bit late here. Despite all the criticism, please be assured that I appeciate the huge effort you've been pulling, I have focused on what I think could be improved ;). I'll make more comments later on, when I'll have digested your library a bit better. Regards, Julien