Opened 9 years ago

Closed 9 years ago

Last modified 13 months ago

#9278 closed patch (wontfix)

Reverse Iterator

Reported by: SF/nicogarnier Owned by: fingolfin
Priority: normal Component: --Other--
Keywords: Cc:
Game:

Description

It was not the most difficult development I ever did but it was in the todo list... Reverse iterators are implemented.

Ticket imported from: #3158890. Ticket imported from: patches/1383.

Attachments (1)

ReverseIterator.patch (1.8 KB ) - added by SF/nicogarnier 9 years ago.

Download all attachments as: .zip

Change History (6)

by SF/nicogarnier, 9 years ago

Attachment: ReverseIterator.patch added

comment:1 by dreammaster, 9 years ago

I hope that this isn't a case of the TODO being out of date, and you doing redundant work, but doesn't the current iterator framework work fine in reverse if we use reverse_begin() and simply compare against end() in a loop?

comment:2 by fingolfin, 9 years ago

This patch seems incomplete, too. It adds a ReverseIterator class, but no way to obtain an instance of that. Moreover, there is no ConstReverseIterator.

I am not quite sure why you found this patch so difficult (or maybe that was irony?) -- it's just a matter of switching _next and _prev in some places, isn't it? :)
In fact, this is why in the standard C++ library (or at least in the GNU implementation thereof), there is a generic reverse_iterator template, which can be wrapped around any concrete (const_)iterator type, to yield a reverse iterator. It simply implement a ++ method which invokes the -- method of the wrapped iterator, etc. etc.. I think that *if* we add reverse iterators, we should follow that way, as it reduces code duplication, and gives us reverse iterators for all container types at once (well, at least all with bidirectional iterators; so the hashmap is not currently covered by this).

I am rejecting this patch for now, as it does not provide any actual functionality and is not the way to go. Should you still be interested on working on proper reverse iterator support, though, I'd be happy to coach you on the subject.

comment:3 by fingolfin, 9 years ago

Status: newpending

comment:4 by fingolfin, 9 years ago

Owner: set to fingolfin
Resolution: wontfix
Status: pendingclosed

comment:5 by digitall, 13 months ago

Component: --Other--
Note: See TracTickets for help on using tickets.