Remove algorithms implementation duplications

François Dumont frs.dumont@gmail.com
Thu Jul 19 19:56:00 GMT 2012


On 07/18/2012 06:50 PM, Marc Glisse wrote:
> On Wed, 11 Jul 2012, François Dumont wrote:
>
>>    Here is the new patch based on Marc Glisse idea to use functors 
>> taking iterators rather than iterators::reference. Compare to the 
>> previous proposal there is no more impact on the creation of 
>> temporaries instances, no additional copy or move constructor. Of 
>> course there are additional iterators or functors copies but those 
>> types are supposed to be lightweight types easy to copy.
>
> I may have shot myself in the foot with that one ;-) (I sometimes have 
> heavy iterators and functors, but that's my problem).
>
> I can't review the patch, but I have a question, if you have time: 
> what is your personal feeling on which approach is simpler, which 
> yields the cleanest code, which is more flexible? Or are they all 
> roughly the same?
> You spent quite a bit of time implementing several versions of this 
> patch, and I would be happy to learn from your experience.
>
You are right, I should have given my feelings about this patch at the 
first place.

     I definitely prefer this latest version. With the previous patch I 
already applied and this one I realized that it is a good practice to 
dereference iterators as late as possible mostly because you do not have 
to care about perfect forwarding anymore. I even apply this rule in the 
__unguarded_partition implementation, it used to take a const _Tp& 
__pivot parameter, now it takes a _RandomAccessIterator iterator type 
and I dereference it only when I need to compare it. Of course, doing 
so, iterators might be dereference more often than they are currently. 
Once again I considered that to dereference an iterator shall be a 
trivial operation. However I try to limit this side effect in some 
functors of __gnu_cxx::__ops; in _Iter_bind2nd_Iter_bind1st_iter and 
_Iter_equals_iter I cache the result of the dereferencing. Saying so, I 
might consider using the same technique for __unguarded_partition which 
is the only place where the number of dereference of iterators is more 
important.

     An other advantage of this approach is that in the internal 
implementations of the algorithm you know exactly what kind of functors 
you are going to deal with, those in __gnu_cxx::__ops. It gives more 
consistency in the approach and so I think it is a little bit more flexible

     I also find the code easier to read but it is mostly because this 
time I took the time to write helper functions that greatly simplified 
the creation and manipulation of the functors. I think I could have come 
to the same result with the previous approach.

     Sorry if you find this feedback rather light, I do not have the 
same C++ background as other libstdc++ maintainers, I feel what I do 
more than I can explain it.

François



More information about the Libstdc++ mailing list