On Wed, 27 Sep 2017 17:41:37 -0700
Brandon Williams <bmw...@google.com> wrote:
> On 09/27, Jonathan Tan wrote:
> > This is similar to using the hashmap in hashmap.c, but with an
> > easier-to-use API. In particular, custom entry comparisons no longer
> > need to be written, and lookups can be done without constructing a
> > temporary entry structure.
> >
> > oidset has been updated to use oidmap.
>
> After a quick glance at the code, I think it looks good. I do have one
> suggestion though. This map is structured much like our internal
> hashmap where if you want to include any data you have to implement your
> own struct with the internal 'entry' struct as the first member in your
> custom struct. I personal dislike this method, despite the potential
> memory savings, because it makes the data structure much more difficult
> to use.
>
> If all i wanted was a map from 'OID -> const char *' then I would need
> to write all the boilerplate code to make a struct which contains the
> map 'entry' struct + the 'const char *' entry.
>
> You are also making the
> caller responsible for allocating the individual entries instead of
> letting the data structure take care of that internally.
In the case where your extra data ("const char *" in your example) fits
in a pointer, yes it's true that the "util" design eliminates the need
to define and allocate a struct. But if you need to store more than
that, you will still have that need.
> To me it seems like a much simpler API for a map would be to just allow
> callers to store a 'void *' as the value.
I agree that the API would be simpler.
My main motivation with this design is indeed to save memory, and not
inconvenience the user too much (in the case where you're storing things
larger than one pointer, you just need to remember to put the special
struct at the beginning of your struct), but if memory is not so
important, I agree that we can switch to the "util" design.
