Re: [PATCH v4 00/13] Add struct strmap and associated utility functions

Elijah Newren [off-list ref] writes:

quoted

It makes sense, but check_dup() sounds like its use pattern would be

        if (check_dup(it) == NO_DUP)
                add(it);

where it is more like "add it but just once".

By the way, is a strset a set or a bag?  If it makes the second add

strset is a set; there is no way to get duplicate entries.

quoted

an no-op, perhaps your check_dup() is what strset_add() should do
itself?  What builtin/shortlog.c::check_dup() does smells like it is
a workaround for the lack of a naturally-expected feature.

Is the expectation that strset_add() would return a boolean for
whether a new entry was added?

It seems to be a reasonable expectation that the caller can tell if
the add was "already there and was a no-op", judging from what we
saw in the shortlog code, which was the first audience the API was
introduced to support.  It seems to benefit from it if it were
available, and has to work around the lack of it with check_dup()
wrapper.