implementation of `Iterator` for `Cursor` is wrong

[hawkw]

I've recently started adding Cursors to intruder_alarm::doubly::List (#9) in a branch. However, I've hit a fairly serious bug in the implementation of Iterator for Cursor. Cursors acting as Iterators will skip the first element of the data structure they cursor over.

The bug

Iterator::next is currently implemented for Cursor by calling self.next_item():

https://github.com/hawkw/alarm/blob/41a3b9d89c9e8a56266fc8a56aaf4cd0e30bc8b6/intruder_alarm/src/cursor.rs#L128-L136

However, Cursor::next_item will advance the cursor to the next element in the list and then call Cursor::get, getting the current element:

https://github.com/hawkw/alarm/blob/41a3b9d89c9e8a56266fc8a56aaf4cd0e30bc8b6/intruder_alarm/src/cursor.rs#L55-L58

In my branch, I'm creating my Cursors pointing at the head element of the list:

https://github.com/hawkw/alarm/blob/d5e1a1002212d2eb511109d5c9f7b3a124683fb8/intruder_alarm/src/doubly/mod.rs#L337-L344

Since the Cursor already points at the head element when Iterator::next is called on it for the first time, Cursor::next_item will advance the cursor and return the next item. The iterator never returns the head element.

How to fix it

We have a couple options here. We could special-case the Iterator impl to check if it's pointing at the first element, but this seems kind of inelegant.

I think a better option is to rewrite Iterator::next for Cursor to call self.get(), store the return value in a temporary variable, call self.move_forward(), and then return the result of the call to get. If we thought this functionality might be useful in contexts other than in the Iterator implementation, we could also consider encapsulating it in a new function on Cursor (maybe called get_and_advance or something?), but this is probably unnecessary.

[hawkw]

This should be a pretty easy fix if anyone's interested in picking it up!

[amanjeev]

Given that I am doing #8, which copies stuff from #9, it is also bound to have the same bug?

If yes, then do you suggest I, perhaps, do this one first? Please advise.

[hawkw]

@amanjeev I wouldn't worry about this right now --- fixing this bug shouldn't require any changes to the doubly-linked list code that you're basing your implementation on.

I'd focus on implementing the basic singly-linked list data structure, and hold off on adding iterators and/or cursors at this point. Whether or not a singly linked list should even provide a cursor (it doesn't really make sense, since a cursor would only be able to traverse the list in one direction) is an open question at this point.