knighton / sparsehash

Automatically exported from code.google.com/p/sparsehash
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Feature request: get deleted_key and empty_key #46

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Implement accessors for deleted and empty_key.

Original issue reported on code.google.com by sjackman on 11 Dec 2009 at 6:05

GoogleCodeExporter commented 9 years ago
Shaun has offered to write up a patch.  Yay!

Shaun, can you also (e)sign the contributor license at
   http://code.google.com/legal/individual-cla-v1.0.html
?

Thanks,
craig

Original comment by csilv...@gmail.com on 11 Dec 2009 at 10:00

GoogleCodeExporter commented 9 years ago
Hi Craig,

Attached is a patch to implement deleted_key and empty_key. I've added a very 
simple
unit test to make sure that all six cases at least compile. I haven't yet added
documentation.

On Dec 9, 6:07 pm, Craig Silverstein <csilv...@google.com> wrote:
...
> I guess we'll have to figure out reasonable answers for these to give
> when the deleted key and empty key haven't been set yet.

I'm asserting that use_deleted or use_empty are true, which is a nice simple
solution. Another possibility is to return
pair<key_type, bool>(delkey, use_deleted)

Cheers,
Shaun

2009-12-31  Shaun Jackman  <sjackman@gmail.com>

    * google/sparsehash/densehashtable.h (empty_key, deleted_key): New function.
    * google/dense_hash_map: Ditto.
    * google/dense_hash_set: Ditto.
    * google/sparsehash/sparsehashtable.h (deleted_key): New function.
    * google/sparse_hash_map: Ditto.
    * google/sparse_hash_set: Ditto.
    * hashtable_unittest.cc (test_int, TestOperatorEquals): Add unit tests.

Original comment by sjackman on 31 Dec 2009 at 8:46

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the patch!  I applied it successfully and tested it, and all looks 
good.  I 
made a few minor changes: mostly formatting, but also making the new methods 
const in 
the wrapper classes (they were already const in foohashtable.h, but not in 
foo_hash_{map,set}).

I'll have one more pair of eyes look it over here, and assuming all goes well, 
it will 
be in the next release.

Original comment by csilv...@gmail.com on 4 Jan 2010 at 9:34

GoogleCodeExporter commented 9 years ago
One more thing: can you provide a patch to the doc files, to document these new 
methods, in the appropriate places (if any)?  I haven't looked yet to see where 
they 
might best fit.

Original comment by csilv...@gmail.com on 4 Jan 2010 at 9:46

GoogleCodeExporter commented 9 years ago
Thanks for catching the const-ness. I missed that. Are you happy with the
assert(use_deleted)? I'll work on documentation next.

Cheers,
Shaun

Original comment by sjackman on 4 Jan 2010 at 9:52

GoogleCodeExporter commented 9 years ago
Yes, I think the asserts are fine -- we get to decide the semantics of these 
functions, which is always nice. :-)  Just be sure to document that it's 
illegal to 
call foo_key before set_foo_key.

Original comment by csilv...@gmail.com on 5 Jan 2010 at 1:52

GoogleCodeExporter commented 9 years ago
This should be resolved in sparsehash 1.6, just released.

Original comment by csilv...@gmail.com on 11 Jan 2010 at 10:53