Re: [boost] Interest for a perl6-like-junctions-class?

From: Rob Stewart (stewart_at_[hidden])
Date: 2005-08-07 16:03:36

From: FlSt_at_[hidden]
> 
> 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_at_[hidden]
Software Engineer                     http://www.sig.com
Susquehanna International Group, LLP  using std::disclaimer;