From: FlSt@gmx.de
I uploaded a new version to the sandbox file vault (junction/boost_junction_v10.zip). There was a bug in my detail::binary_op_type struct. I removed it ;-) Now all my test programs give the same results as Rob's implementation. An now it compiles faster .
I grabbed v11 which you posted by the time I looked. This is the first time I've studied your code and I have some comments: 0. Your *junction classes need to be in your junctions namespace. I know Pavel questioned why they weren't in the detail namespace, but it is reasonable to think that some will want to create a variable to hold such an object for repeated comparisons. 1. Aside from the junction type in the base class, plus naming and stylistic differences, your library looks a great deal like mine. 2. You don't have iterator range support yet. 3. Your detail::do_comparison() functions would be clearer if named "compare," don't you think? 4. I see that you've made good use of implementing one type in terms of another. I did that early on in my operator intensive version and ignored it after refactoring my library. I need to revisit that. 5. Your n_m_junction do_comparison() tests against count <= M in the return statement unnecessarily. The loop is terminated with a return if count ever exceeds M. 6. Do you like | better than ^ as the user-predicate delimiter? I didn't even examine the operator precedence when selecting one. ^ is higher than |, but both are higher than && and ||. That will force the use of parentheses in many expressions. Perhaps we should consider the comma. 7. Your junction class template parameterized with the various junction classes is an interesting approach. It allows you to select the operation based upon the junction type while keeping the operators strictly in terms of that class template. 8. I think its interesting to use member operators to handle the junction on the LHS plus the non-member operators to handle the non-junction on the LHS/junction on the RHS case. That's possible for your implementation because you do everything based upon the junction type. 9. You have no operator << support. I figure that folks will want to be able to stream these objects if they create instances (see item 0). 10. Your param_type nested typedef is nice. I had to switch to taking everything by reference to const to avoid having to create excessive numbers of templates. 11. You should name the "Range" template parameter "FwdRange" or "ForwardRange" to clarify that it needs to model the ForwardRange concept from Boost.Range. It would be nice if Boost.Range offered a ForwardRangeConcept concept checking class so we could use function_requires< ForwardRangeConcept<FwdRange> >(); in the function templates and BOOST_CLASS_REQUIRE(FwdRange, boost, ForwardRangeConcept); in the class templates to help user's diagnose errors. It's clear that you've adopted many ideas from my mails and library and you've brought many innovative ideas of your own. Great work! -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;