Web lists-archives.com

Re: [PATCH 03/11] sha1-array: provide oid_array_filter




Stefan Beller <sbeller@xxxxxxxxxx> writes:

> +/* Call fn for each oid, and retain it if fn returns 0, remove it otherwise */
> +int oid_array_filter(struct oid_array *array,
> +		     for_each_oid_fn fn,
> +		     void *cbdata);

Comparing this with object_array_filter(), which I think this was
modeled after, I notice that this is much harder to remember how to
use it correctly.

    void object_array_filter(struct object_array *array,
                             object_array_each_func_t want, void *cb_data)

It is clear to a potential user of this function who needs to
implement the callback function what the function should return,
because it calls it "want" (as opposed to "fn").  The function must
answer "I want to keep this?  Yes/no?".

Also polarity of oid_array_filter()'s callback is different.  It
retains elements for which "fn" returns false.

So one step of improvement may be to rename "fn" to "reject", but as
we do not have any caller of this new function yet, perhaps match
the polarity and call it "want"?

I think "x_filter" is perhaps trying to match other language's
filter(), e.g. Python's

	>>> filter(lambda x: x % 2 == 0, range(0,10))
	[0, 2, 4, 6, 8]

so "say yes if you want to keep it" may be more familiar than "say
yes if you want to filter it out".