diff --git a/adr/001_interface_design.md b/adr/001_interface_design.md index 0ba381c..b75df85 100644 --- a/adr/001_interface_design.md +++ b/adr/001_interface_design.md @@ -1,21 +1,21 @@ -# Designing an Idiomatic Interface +# Designing an Idiomatic API Interface -Currently, the contract for package was built without design. -More attention was paid to implementing the underlying functionality of the cuckoo hashing. +We (the maintainers) built `go-cuckoo`'s API interface without design intent. +Up until now, we paid more attention implementing the underlying functionality of the cuckoo hashing. -With the fundamentals of the algorithm built, our API contract should be revisited. +With the fundamentals of the algorithm built, we should revisit the interface. It should align closer to the following principles: -- **Congruency to the builtin map.** - Our cuckoo table should have the same core functionality as Go's built-in map. +- **Congruency** + A `go-cuckoo` table should have the same core functionality as Go's built-in map. -- **Familiarity to the builtin map.** - If our cuckoo table behaves similarly to Go's standard map, our user will intuitively know how to use it. - This lowers the cognitive load our developers must carry. +- **Familiarity** + A `go-cuckoo` table should behave similarly to Go's standard map, so users will intuitively know how to use it. + In effect, its users will carry less cognitive load. ## Current State -### Interface of the Builtin Map +### Interface of the built-in Map Listed below is every interface provided by Go to the built-in map object. Also included, are the functions from the package `maps` in the standard library. @@ -23,7 +23,7 @@ Also included, are the functions from the package `maps` in the standard library
Interfaces -| # | Builtin Interface | Description | +| # | built-in Interface | Description | | --- | ---------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | | 1 | `m := make(map[K]V)` | Returns an empty map using the built-in `make()` function. | | 2 | `m := make(map[K]V, hint)` | Returns an empty map using `make()`, with a capacity 'hint'. This hint is how many items the map expects to hold, _not_ a measure of how large it is. | @@ -56,7 +56,7 @@ On the other hand, here is the current contract for `go-cuckoo`.
Interfaces -| # | Builtin Interface | Description | +| # | built-in Interface | Description | | --- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | | 1 | `m := New(opts...)` | Creates a table using the default hash and equal function. The options configure its behavior. Confined to comparable keys. | | 2 | `m := NewBy(keyFunc, opts...)` | Like #1, but allows any key type. A `keyFunc` is used to derive a comparable key. | @@ -311,34 +311,22 @@ There is no analog. ### Solving Congruency -The following changes will be made to accomodate for congruency: +We should make the following changes to accomodate for congruency:
ok := maps.EqualFunc(m1, m2, fn) -To solve this, we need a new function: +We should implement a new function: ```go -func EqualFunc[K, V1, V2 any](t1 *Table[K, V1], t2 *Table[K, V2], eq func(V1, V2) bool) bool { - if t1.Size() != t2.Size() { - return false - } - - for k, v1 := range t1.Entries() { - if v2, ok := t2.Get(k); !ok || eq(v1, v2) { - return false - } - } - - return true -} +func EqualFunc[K, V1, V2 any](t1 *Table[K, V1], t2 *Table[K, V2], eq func(V1, V2) bool) bool ``` This function is free, and not bound as a receiver function. (It is called `cuckoo.Equal(t1, t2)`, not `t1.Equals(t2)`.) The latter implies `t1` has authority, when in fact neither do. -Equality will be defined as: +We define equality as: 1. Neither table has a key the other doesn't. 2. Each key has the same value in each table. @@ -353,22 +341,20 @@ So, we must assume that: The name `EqualFunc` is already taken by `EqualFunc[K, V]`: an alias for `func(a, b K) bool`. Inlining `EqualFunc[K, V]` would solve this problem. -The documentation attached to it would be moved to `DefaultEqualFunc`. +We will move the documentation attached to it to `DefaultEqualFunc`.
ok := maps.Equal(m1, m2) -The addition of `cuckoo.EqualFunc` makes an implementation trivial: +We should implement a new function, to conform with the standard library: ```go -func Equal[K any, V comparable](t1, t2 *Table[K, V]) bool { - return EqualFunc(t1, t2, DefaultEqualFunc[V]) -} +func Equal[K any, V comparable](t1, t2 *Table[K, V]) bool ``` -To conform with the standard library, a new function should be added. +It uses the same equality check as in `EqualFunc`. Once again, the function is free because it is symmetric.
@@ -376,18 +362,10 @@ Once again, the function is free because it is symmetric.
maps.Insert(m, seq) -This functionality requires a new receiver: +We should implement a new receiver for the table: ```go -func (t *Table[K, V]) Insert(seq iter.Seq2[K, V]) error { - for k, v := range seq { - if err := t.Put(k, v); err != nil { - return err - } - } - - return nil -} +func (t *Table[K, V]) Insert(seq iter.Seq2[K, V]) error ``` A receiver fits better even though `maps.Insert` is a free function, because copying it is asymmetric. @@ -404,21 +382,20 @@ Ultimately, `t.Insert()` is a better choice to be consistent with `maps`.
maps.Copy(dst, src) -To solve this, we must implement a new receiver. -Luckily, `t.Insert` makes it trivial: +We should implement a new receiver for the table: ```go -func (t *Table[K, V]) Copy(src *Table[K, V]) error { - return t.Insert(src.Entries()) -} +func (t *Table[K, V]) Copy(src *Table[K, V]) error ``` +It's functionality should match that of `t.Insert()`. + A receiver fits better even though `maps.Copy` is a free function, 'copying' it is asymmetric: `dst` is writen into by `src`. It is only free because Go's standard map is built into the language, and so cannot have receivers. The name `t.Merge()` might be more accurate, but it does work because: -- `t.Copy()` matches Go's builtin `copy()`, and `io.Copy()`. The Go team used [the same logic](https://github.com/golang/go/discussions/47330#discussioncomment-1167799) to name `maps.Copy()`. +- `t.Copy()` matches Go's built-in `copy()`, and `io.Copy()`. The Go team used [the same logic](https://github.com/golang/go/discussions/47330#discussioncomment-1167799) to name `maps.Copy()`. In this case, `t.Merge()` would be an outlier. - `t.Merge()` implies some sort of conflict-resolution, when there is not. It simply overwrites the values. @@ -428,16 +405,10 @@ The name `t.Merge()` might be more accurate, but it does work because:
maps.DeleteFunc(m, fn) -A few function can fill this gap: +We should implement a new receiver for the table: ```go -func (t *Table[K, V]) DeleteFunc(del func(K, V) bool) { - for k, v := range t.Entries() { - if del(k, v) { - t.Drop(k) - } - } -} +func (t *Table[K, V]) DeleteFunc(del func(K, V) bool) ``` It would have the same functionality as `maps.DeleteFunc`. @@ -452,15 +423,22 @@ The word `Delete` is also convention, tying back to the built-in `delete()`.
m := maps.Collect(seq) -This functionality would benefit from a new constructor. -Luckily, `t.Insert` makes this easy: +We should implement a new constructor. ```go -func Collect[K comparable, V any](seq iter.Seq2[K, V]) (*Table[K, V], error) { - t := New[K, V]() - err := t.Insert(seq) - return t, err -} +func Collect[K comparable, V any](seq iter.Seq2[K, V]) (*Table[K, V], error) +``` + +It would create a `New()` table, and insert all entries in `seq`. + +This reveicer only supports the standard table constructor, with comparable keys. +It is tempting to add `CollectBy` or `CollectCustom` to support all table types, but doing so would pollute the public interface. + +It would be just one more line to initialize the table and then call `t.Insert` directly: + +```go +t := // ... +err := t.Insert(seq) ```
@@ -468,10 +446,97 @@ func Collect[K comparable, V any](seq iter.Seq2[K, V]) (*Table[K, V], error) {
m := map[K]V{...} -This functionality is complicated, because entries are generic; their addition cannot be through table options. -A new constructor must support this functionality. +We should make a new constructor, because entries are generic. +So, creating an option with inialized entries doesn't work. -Should it support options or custom hashes or `keyFunc`'s? -No, because +With the previous additions, users have a few options. +If they want to use a `New()` table, `t.Collect` matches well: + +```go +t, err := cuckoo.Collect(func(yield func(K, V) bool) { + yield(key1, val1) + yield(key2, val2) +}) +``` + +For `NewCustom()` or `NewBy()` tables, users can call `t.Insert` after initialization: + +```go +t := // ... +err := t.Insert(func(yield func(K, V) bool) { + yield(key1, val1) + yield(key2, val2) +}) +``` + +It is one more line. +But, the alternative is polluting the public interface with corresponding `*WithEntries` constuctors. + +
+ +
+m := make(map[K]V, hint) + +We should add a new option: + +```go +func ExpectedSize(n int) Option +``` + +When fed to a table, it will allocate enough space to hold `n` entries without a resize. + +
+ +
+clear(m) + +We should implement a new receiver: + +```go +func (t *Table[K, V]) Clear() +``` + +It will remove all entries from the table. + +
+ +
+m2 := maps.Clone(m) + +We should implement a matching function: + +```go +func (t *Table[K, V]) Clone() *Table[K, V] +``` + +Also, it will copy the hash, equality function, and options used in the table. + +
+ +
+it := maps.Keys(m) + +We should implement a matching function: + +```go +func (t *Table[K, V]) Keys() iter.Seq[K] +``` + +It is tempting to just have `All()`, but it returns a `Seq2`, not a `Seq`. +There is no iterator adaptor between `Seq` and `Seq2`, and will not be for the foreseeable future. +This function, while it feels superfluous, is required. + +
+ +
+it := maps.Values(m) + +We should implement a matching function: + +```go +func (t *Table[K, V]) Values() iter.Seq[V] +``` + +For the same reason we need `Keys()`, we also need `Values()`.