On Thu, May 11, 2017 at 02:35:17PM -0700, Jonathan Nieder wrote:

> This structure could be simplified by putting the lazy-initializing
> tip_oids lookup in a separate function.  For example:
> 
>       int tip_oids_contain(struct oidset *tip_oids,
>               struct ref *unmatched, struct ref *newlist,
>               const struct oid *id)
>       {
>               if (oidset_is_empty(tip_oids)) {
>                       add_refs_to_oidset(tip_oids, unmatched);
>                       add_refs_to_oidset(tip_oids, newlist);
>               }
>               return oidset_contains(tip_oids, id);
>       }

Yeah, I started to write it that way, but in the empty-ref case it will
try to add the empty refs over and over. I guess that's not that big a
deal, though, as we know the noop add is O(1) then. :)

> That way, the caller could be kept simple (eliminating can_append
> and the repeated if).

Yes, shoving it all into the sub-function is a big win for readability.

-Peff

Reply via email to