Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding methods and changing the API in goish way #1

Merged
merged 6 commits into from
Aug 21, 2018

Conversation

shenwei356
Copy link
Contributor

@shenwei356 shenwei356 commented Aug 21, 2018

Hi @brentp ,

This PR includes:

  1. Adding methods:
    • Del for deleting a key-value pair.
    • Keys returns a channel for iterating keys.
    • Items returns a channel for iterating all key-value pairs.
    • Size returns the map size.
  2. Fixing a bug:
  3. Changing APIs in goish way
    • Get: Function in Java does not supports returning multiple values, so the original implemention uses a NO_VALUE to indicating keys no found. But golang does, so we can just returns another boolean value like builtin map does (value, ok := m[key]).
    • Put does not need returning value.
  4. And of cause updated tests and README.

@brentp
Copy link
Owner

brentp commented Aug 21, 2018

awesome! can you verify that the benchmarks are unaffected by the switch to val, ok ?

@shenwei356
Copy link
Contributor Author

It seems unaffected.

Previous

BenchmarkIntIntMapFill-16                             10         147056031 ns/op        402522201 B/op        24 allocs/op
BenchmarkStdMapFill-16                                 5         290888868 ns/op        95069708 B/op      73641 allocs/op
BenchmarkIntIntMapGet10PercentHitRate-16           10000            115791 ns/op           40252 B/op          0 allocs/op
BenchmarkStdMapGet10PercentHitRate-16              10000            108940 ns/op            9504 B/op          7 allocs/op
BenchmarkIntIntMapGet100PercentHitRate-16            500           2311054 ns/op          805044 B/op          0 allocs/op
BenchmarkStdMapGet100PercentHitRate-16               100          10592906 ns/op          950572 B/op        735 allocs/op

Now

BenchmarkIntIntMapFill-16                             10         141656399 ns/op        201195776 B/op        22 allocs/op
BenchmarkStdMapFill-16                                 5         287342524 ns/op        95065142 B/op      73611 allocs/op
BenchmarkIntIntMapGet10PercentHitRate-16           20000             59330 ns/op           10059 B/op          0 allocs/op
BenchmarkStdMapGet10PercentHitRate-16              10000            130110 ns/op            9503 B/op          7 allocs/op
BenchmarkIntIntMapGet100PercentHitRate-16            500           2630494 ns/op          402394 B/op          0 allocs/op
BenchmarkStdMapGet100PercentHitRate-16               100          10598248 ns/op          950077 B/op        732 allocs/op

@brentp brentp merged commit 931866a into brentp:master Aug 21, 2018
@brentp
Copy link
Owner

brentp commented Aug 21, 2018

cheers. this is a great improvement to the API.

@shenwei356
Copy link
Contributor Author

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants