
Please explicitly state in your review whether the library should be accepted.
YES, this library should be accepted after some issues are solved.
- What is your evaluation of the design?
The names of include files are not easy to remember because they are not fully conformant to the macro name (sometimes yes, sometimes not): - HAS_TYPE is in type.hpp - HAS_TEMPLATE is in template.hpp - HAS_TEMPLATE_CHECK_PARAMS is in template_params.hpp -> template_check_params.hpp? - HAS_MEMBER_DATA is in mem_data.hpp -> member_data.hpp? - HAS_MEMBER_FUNCTION is in mem_fun.hpp -> member_function.hpp? I would use the lower case version of the macro with BOOST_TTI_HAS_ removed so that it is easy to remember. I agree with others that traits should be generated in the local namespace instead of boost::tti. Then the user has full control of name clash.
- What is your evaluation of the implementation?
I have not been in detail in the implementation. Just opening some files and this is what came out: can you remove trailing spaces?
- What is your evaluation of the documentation?
- simple examples should appear very early in the doc. You could maybe
add a tutorial section starting with the simplest examples, leaving
the more complex to the end.
You should also give an application example of the trait itself.
for example (untested):
template
We could just have easily used the boost::tti::mf_valid_member_type metafunction to the same effect: boost::tti::mf_valid_member_type < DTypeNM
"
I would be happy to see simple examples like the following:
BOOST_TTI_HAS_TYPE(B)
BOOST_TTI_HAS_TYPE(mytrait, B)
struct A {
struct B { }
};
boost::tti::has_type_B<A>::value; // true
boost::tti::has_type_B<int>::value; // false
boost::tti::has_type_B<B>::value; // false
boost::tti::mytrait<A>::value; // true
boost::tti::mytrait<int>::value; // false
boost::tti::mytrait<B>::value; // false
- typos:
. in tti_detail.html: "will tell use" -> "will tell us" or "will
tell the user"
. in tti_nested_type.html: "The goal of the TTI library is never to
produce a compiler" -> "The goal of the TTI library is to never
produce a compiler"
- I have seen no explanation of the implementation behavior with
regards to public/protected/private category of inner type/member.
Could give details in the documentation? I imagine there is no need to
introspect private/protected types/members. But what happens if the
type/member exists but is private? Compilation error or value==false
returned? By the way, I have seen that this is currently not tested as
all data and types are public in the tests (in structs).
I have tested the following program:
#include <iostream>
#include
- What is your evaluation of the potential usefulness of the library?
I am sure that it can be usefull to implement the best algorithms to appropriate types. For example, you can imagine that adding an element at the front of a container could be specialized for containers that have member function push_front by using it and for other containers by using more complex copying algorithm. Then the best algorithm is used tranparently to the final user.
- Did you try to use the library? With what compiler? Did you have any problems?
Yes, I tried to use the library with g++ 4.4.4. It worked as expected apart for private members as reported (tested HAS_TYPE and HAS_MEMBER_DATA). I ran tests with intel 11 and 12 which failed but the author is dealing with this. I ran tests with g++ 4.4.4 and nothing failed (tested tti target only).
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read the doc + some testing (about 4 hours).
- Are you knowledgeable about the problem domain?
I am a bit aware of metaprogramming as I am trying to add an extension to the type trait library for detecting existence of operators.
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
YES. Frédéric