Re: [PATCH 6/9] oid-array: provide a for-loop iterator

From: Taylor Blau <me@ttaylorr.com>
Date: Dec 4, 2020, 19:11 UTC
Message-ID: <X8qJ9yKVcGSw0bWx@nand.local>
In-Reply-To: <X8qIaOe3aT1qr+es@nand.local>

On Fri, Dec 04, 2020 at 02:05:12PM -0500, Taylor Blau wrote:

Show 15 quoted lines

> 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.