S2ClosestEdgeQueryBase::InitCovering: Use a single iterator

[benguild]

This is sibling to: golang/geo#248
While resolving a bug with the code sync between Go/C++ (golang/geo#247) I noticed the TODO.

Since there's seemingly an effort to mirror implementations, I'm opening a draft PR, because I barely work with C++ and I'd need someone to take a look at this.

[jmr]

@ericveach

[benguild]

Due to the complexity of this and the Go version, it'd help to have the feedback about whether or not anyone has thoughts on how to improve it, or if the TODO from the current code should just be removed from both Go and C++ otherwise.

I'm happy to take another look but I can't keep this in mind forever and would need to move onto other things if there's no bandwidth to consider the changes.

[jmr]

but I can't keep this in mind forever

If it's going to be a problem for you to come back to the code in a few days or weeks, then it's definitely going to be a problem for someone else to look at it in a few years. Can you make sure the code is sufficiently documented so you don't need to "keep it in mind"?

I will release the benchmarks soon, so you can test whether this makes a difference.

Funny that it's so much more complex in go than C++. What's that?

[benguild]

If it's going to be a problem for you to come back to the code in a few days or weeks, then it's definitely going to be a problem for someone else to look at it in a few years. Can you make sure the code is sufficiently documented so you don't need to "keep it in mind"?

To clarify, I can't guarantee I won't be blocked by future events.

Funny that it's so much more complex in go than C++. What's that?

Go's patterns are different, but the C++ may not be totally correct as mentioned since I don't work with it often. (hence the "draft" PR and RFC)