Re: [PATCH v4] fetch-pack: always allow fetching of literal SHA1s

[Jeff King <peff@peff.net>]

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