Re: [PATCH 3/6] connected: refactor iterator to return next object ID directly

Derrick Stolee [off-list ref] writes:

On 8/20/2021 6:08 AM, Patrick Steinhardt wrote:

quoted

The object ID iterator used by the connectivity checks returns the next
object ID via an out-parameter and then uses a return code to indicate
whether an item was found. This is a bit roundabout: instead of a
separate error code, we can just retrun the next object ID directly and
use `NULL` pointers as indicator that the iterator got no items left.
Furthermore, this avoids a copy of the object ID.

Refactor the iterator and all its implementations to return object IDs
directly. While I was honestly hoping for a small speedup given that we
can now avoid a copy, both versions perform the same. Still, the end
result is easier to understand and thus it makes sense to keep this
refactoring regardless.

It's too bad about the lack of measurable performance gains, but the
new code _is_ doing less, it's just not enough.

I agree that the new code organization is better.

The only potential downside I can think of is the loss of ability to
convey the reason why it failed to return one by adding new return
codes from the function, which I do not immediately see all that
useful future extension anyway, so I agree that "we find what we
found, or NULL if we don't find" is much straight-forward and easier
to understand.

Nicely done.

Thanks.