On Fri, Dec 04, 2020 at 02:05:12PM -0500, Taylor Blau wrote:
> On Fri, Dec 04, 2020 at 01:53:23PM -0500, Jeff King wrote:
> > I also considered adding a full iterator type with init/next/end
> > functions (similar to what we have for hashmaps). But it ended up making
> > the callers much harder to read. This version keeps us close to a basic
> > for-loop.
>
> Yeah, I think that a full-blown iterator type is overkill for this
> purpose. Another possible approach could be a macro:
>
> #define for_each_oid_array_unique(arr, i) \
> for (i = 0; (i) < (array)->nr; i = oid_array_for_each_unique((arr), i))
>
> but I don't think that's making anything more clear than
> 'oid_array_for_each_unique' already is. So, I like the approach that you
> took here.
Hmm. I take part of that back; it would simplify the caller in the eighth
patch, which first wants to sort the list before iterating it (which is
necessary, because we look at 'i = 0' before calling
oid_array_for_each_unique()).
So this could look instead like:
#define for_each_oid_array_unique(arr, i) \
for (oid_array_sort(), i = 0; (i) < (array)->nr; i = oid_array_for_each_unique((arr), i))
Maybe a little too magical, who knows.
