From: Florian Stegner <FlSt@gmx.de>
Rob Stewart wrote:
From: FlSt@gmx.de
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.
Thats a good point. Hmm, but how should a client declare them?
set<int> a; junction< disjunction< set< int > > > a_junction = all_of( a );
looks a little confusing. I would prefer something like
disjunction< set< int > > a_junction; But this is not compatible with my junction<> - class. Do you have an idea?
Nah, like this: set<int> a; all_values<set<int> > all_of_a(all_of(a)); I've nearly got that ready plus I'm using your dispatch technique. I'll upload it when I get it compiling.
1. Aside from the junction type in the base class, plus naming and stylistic differences, your library looks a great deal like mine.
It was a little funny: After I fixed my compilation time problems I looked at your implementation and saw that it was very similar to mine. Then I tried to bring the best ideas of both implementations together.
Collaboration at its best, I'd say.
2. You don't have iterator range support yet.
This will be the next point I will work on.
Take a look at mine. My next version will follow your dispatch technique, so you might just want to wait for that one.
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.
That was the thing which makes the 10% runtime differences between our implementations without optimizing.
Mine is 10% faster or slower because of that? I've lost track of which is faster now.
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.
Ok you are right, when you say it's important to examine the operator precedence. But the comma operator looks confusing:
all_of(a) ,equal(), any_of(b).
You could get used to it, if you had to.
From the readability viewpoint I like the bitwise-or operator | more than the bitwise-xor operator ^.
But now I'm a little bit confused. The precedence must be higher, or I'm wrong? For example someone writes: any_of( a ),equal(),all_of( b ) && any_of( c ),equal(),all_of(d); So all_of( b ) && any_of( c ) will be evaluated before the ",". If you write any_of( a )|equal()|all_of( b ) && any_of( c )|equal()|all_of(d) It will give the correct answer.
That's a good point. I was thinking about the lambda expressions one might write for the predicate. I suppose the use case you've mentioned is more important, so we should have higher precedence than the logical operators.
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).
I removed them, but it's no problem to add them again. I thought, there was no need for them. Ok could be useful for debugging.
I'm thinking they would be useful for any context in which one might wish to stream a container. Yes, that includes debugging, but there are others.
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.
I think this is the correct forum to ask for such a feature ;-)
(I'm hoping that Thorsten will see mention of his library and take note. ;-) -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;