FoundationDB

Golang bindings - RangeIterator.Get() & side effects?

bindings

(Mikael Rapp ) #1

Hi,

I would just like to raise a discussion about RangeIterators and their current names/docs/implementation.

I ran into an issue where multiple calls to RangeIterator.Get() gave different results and even caused an out of bounds panic. The root cause was easy to identify since it has side effects which was not what I expected: https://github.com/apple/foundationdb/blob/master/bindings/go/src/fdb/range.go#L268

Expectation
With 2 methods primary methods (Advance and Get), I would expect Advance() to progress the iterator and Get() to fetch whatever is located at the current position. However, Get() is not side effect free and can only be safely called once after each Advance. I would argue that the current naming+behavior does not conform to the “principle of least surprise”.

Suggestion:
The Documentation for Advance() state: “…You must call this before every call to Get or MustGet” but i think this behavior should either be changed or at least clarified in the docs for the Get() method as well.

The Advance method of this RangeIterator must have returned true prior to calling Get.
=>
The Advance method of this RangeIterator must have returned true prior to each call to Get.


(A.J. Beamon) #2

Hmm, I agree this is behavior is not straightforward. I suspect the reason it was done this way was because it made it easy to perform the optimization that we start fetching the next results once you’ve read the last of the currently available results. I think it may be possible to still keep this optimization, though, while also removing the side effect of incrementing the index in Get.

I’ll talk this over with a few folks here and then potentially raise this as an issue in GitHub.