Stefan Beller <sbeller@xxxxxxxxxx> writes:
> When using the hashmap a common need is to have access to arbitrary data
> in the compare function. A couple of times we abuse the keydata field
> to pass in the data needed. This happens for example in patch-ids.c.
It is not "arbitrary data"; it is very important to streess that we
are not just passing random crud, but adding a mechansim to
tailor/curry the function in a way that is fixed throughout the
lifetime of a hashmap.
I haven't looked at the use of keydata in patch-ids.c and friends to
decide if that "abuse" claim is correct; if it were the case, should
we expect that a follow-up patch to clean up the existing mess by
using the new mechanism? Or does fixing the "abuse" take another
mechanism that is different from this one?
> While at it improve the naming of the fields of all compare functions used
> by hashmaps by ensuring unused parameters are prefixed with 'unused_' and
> naming the parameters what they are (instead of 'unused' make it
> 'unused_keydata').
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> diff --git a/hashmap.h b/hashmap.h
> index de6022a3a9..1c26bbad5b 100644
> --- a/hashmap.h
> +++ b/hashmap.h
> @@ -33,11 +33,12 @@ struct hashmap_entry {
> };
>
> typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key,
> - const void *keydata);
> + const void *keydata, const void *cbdata);
As I view the new "data" thing the C's (read: poor-man's) way to
cutomize the function, I would have tweaked the function signature
by giving the customization data at the front, i.e.
fn(fndata, entry, entry_or_key, keydata);
as I think the way we wish to be able to express it in
a better language would have been something like:
(partial_apply(fn, fndata))(entry, entry_or_key, keydata)
But what you did is OK, too.
> +extern void hashmap_init(struct hashmap *map,
> + hashmap_cmp_fn equals_function,
> + const void *data,
> + size_t initial_size);
And this does match my expectation ;-).
