Derrick Stolee <stolee@xxxxxxxxx> writes: > On 8/20/2021 6:08 AM, Patrick Steinhardt wrote: >> 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.
