This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Remove algorithms implementation duplications


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]