Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 199 additions & 0 deletions maputil/bimap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
package maputil

import (
"encoding/json"
"fmt"
"sync"
)

// BiMap "bidirectional map" is a map that preserves the uniqueness of its values as well as that of its keys
type BiMap[K comparable, V comparable] struct {
mu sync.RWMutex
normal map[K]V
reverse map[V]K
}

// Key returns the key for the given value.
func (m *BiMap[K, V]) Key(v V) K {
m.mu.RLock()
defer m.mu.RUnlock()
return m.reverse[v]
}

// Value returns the value for the given key.
func (m *BiMap[K, V]) Value(k K) V {
m.mu.RLock()
defer m.mu.RUnlock()
return m.normal[k]
}

// Keys returns a slice of query keys
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Bayi,

First of all, thank you for the nice and clean code. Also with the good thought on safety (locks).

Here a kind of reflection on your design and implementation for the Keys() and Values() function.
I wonder that these functions do NOT preserve the order of the returned keys corresponding to the order to which the values are given in the input. Is that true?

I see that the tests for the function for example, only test for the existence of each key in the returned set, but not to their order.

I suggest to have the order of the returned slice corresponding to the order of the input slice. This is much more intuitive and allows the caller to know for each key returned whose value it matches to.

Moreover: I also guess that, for this implementation, if you pass as input some value(s) (or key in the Values func) that are not in the BiMap, the return slice will contain only keys(values) for the found elements. This makes it impossible to know the correspondence between input and output values. That is, for which value(key) in the input, the returned key(value) is associated at the output. This would be true even if the order would be preserved -- because there is no indication of which elements are in the BiMap or not.

That all explained, I have two suggestions:

  1. Change the implementation, so that the order of the returned elements correspond to the order of the given elements
  2. Return an error if one of the input elements cannot be found in the BiMap as a value(in the case of the Keys func) or as a key (in the case of the Values func).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two methods have been renamed. Fundamentally, they are wrappers for maputil.FilterByKeys and maputil.FilterByValues, so it is inappropriate to claim that there are errors. Regarding the sorting issue, the inherent nature of the map data structure means it does not care about ordering, so there is no need to support sorted output.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Baiy,

Thanks for the renaming.

About the ordering, you indeed confirm that "the inherent nature of the map data structure means it does not care about ordering...".

I understand. But I do think it creates a design smell. Now, if you ask for the values associated with a certain slice of keys you will receive a slice of values that cannot one by one be associated to the keys you passed. I think the return slice is dubious in how it relates to the the input leading to confusion and possibly code that is difficult to understand.

Per se, not an error, but I would not like to use such an unclear interface.

If you want to continue with the behaviour, that is ok, but I would appreciate if you could evaluate the suggestions given before.

func (m *BiMap[K, V]) Keys(v ...V) []K {
m.mu.RLock()
defer m.mu.RUnlock()
return Keys(FilterByValues(m.normal, v))
}

// Values returns a slice of query values
func (m *BiMap[K, V]) Values(k ...K) []V {
m.mu.RLock()
defer m.mu.RUnlock()
return Values(FilterByKeys(m.normal, k))
}

// AllKeys returns a slice of all keys
func (m *BiMap[K, V]) AllKeys() []K {
m.mu.RLock()
defer m.mu.RUnlock()
return Keys(m.normal)
}

// AllValues returns a slice of all values
func (m *BiMap[K, V]) AllValues() []V {
m.mu.RLock()
defer m.mu.RUnlock()
return Values(m.normal)
}

// ContainsValue returns true if the given value exists.
func (m *BiMap[K, V]) ContainsValue(v V) bool {
m.mu.RLock()
defer m.mu.RUnlock()
_, ok := m.reverse[v]
return ok
}

// ContainsKey returns true if the given key exists.
func (m *BiMap[K, V]) ContainsKey(k K) bool {
m.mu.RLock()
defer m.mu.RUnlock()
_, ok := m.normal[k]
return ok
}

// Put insert the given key-value pair. if it already exists return error
func (m *BiMap[K, V]) Put(k K, v V) error {
m.mu.Lock()
defer m.mu.Unlock()
if _, ok := m.normal[k]; ok {
return fmt.Errorf("key %v already exists", k)
}
if _, ok := m.reverse[v]; ok {
return fmt.Errorf("value %v already exists", k)
}
m.normal[k] = v
m.reverse[v] = k
return nil
}

// PutForce force insert the given key-value pair. if the data exists, it will be deleted and inserted again
func (m *BiMap[K, V]) PutForce(k K, v V) error {
m.mu.Lock()
defer m.mu.Unlock()
// check if exists, if exists remove
if _, ok := m.normal[k]; ok {
delete(m.reverse, m.normal[k])
delete(m.normal, k)
}
if _, ok := m.reverse[v]; ok {
delete(m.normal, m.reverse[v])
delete(m.reverse, v)
}
m.normal[k] = v
m.reverse[v] = k
return nil
}

// RemoveKey removes a key-value pair from the map by key
func (m *BiMap[K, V]) RemoveKey(k K) {
if !m.ContainsKey(k) {
return
}
m.mu.Lock()
defer m.mu.Unlock()
delete(m.reverse, m.normal[k])
delete(m.normal, k)
}

// RemoveValue removes a key-value pair from the map by Value
func (m *BiMap[K, V]) RemoveValue(v V) {
if !m.ContainsValue(v) {
return
}
m.mu.Lock()
defer m.mu.Unlock()
delete(m.normal, m.reverse[v])
delete(m.reverse, v)
}

// Clear removes all key-value pairs from the map
func (m *BiMap[K, V]) Clear() {
m.mu.Lock()
defer m.mu.Unlock()
m.normal = make(map[K]V)
m.reverse = make(map[V]K)
}

// Len returns the number of key-value pairs in the map
func (m *BiMap[K, V]) Len() int {
return len(m.normal)
}

// Inverse returns a new BiMap with the keys and values swapped
func (m *BiMap[K, V]) Inverse() *BiMap[V, K] {
r, _ := NewBiMapFromMap(m.reverse)
return r
}

// ToMap returns a map
func (m *BiMap[K, V]) ToMap() map[K]V {
return m.normal
}

// MarshalJSON implements the json.Marshaler interface.
func (m *BiMap[K, V]) MarshalJSON() ([]byte, error) {
return json.Marshal(m.normal)
}

// UnmarshalJSON implements the json.Unmarshaler interface.
func (m *BiMap[K, V]) UnmarshalJSON(data []byte) error {
normal := make(map[K]V)
if err := json.Unmarshal(data, &normal); err != nil {
return err
}
return m.fromMap(normal)
}

func (m *BiMap[K, V]) fromMap(d map[K]V) error {
m.mu.Lock()
defer m.mu.Unlock()
if m.normal == nil || m.reverse == nil {
m.normal = make(map[K]V)
m.reverse = make(map[V]K)
}
for k, v := range d {
if _, ok := m.reverse[v]; ok {
return fmt.Errorf("value %v already exists", v)
}
m.normal[k] = v
m.reverse[v] = k
}
return nil
}

// NewBiMap creates a new BiMap
func NewBiMap[K comparable, V comparable]() *BiMap[K, V] {
return &BiMap[K, V]{
normal: make(map[K]V),
reverse: make(map[V]K),
}
}

// NewBiMapFromMap creates a new BiMap from a map
func NewBiMapFromMap[K comparable, V comparable](d map[K]V) (*BiMap[K, V], error) {
biMap := NewBiMap[K, V]()
if err := biMap.fromMap(d); err != nil {
return nil, err
}
return biMap, nil
}
176 changes: 176 additions & 0 deletions maputil/bimap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package maputil

import (
"encoding/json"
"testing"

"github.com/duke-git/lancet/v2/internal"
"golang.org/x/exp/slices"
)

func TestBiMap_Get(t *testing.T) {
biMap, _ := NewBiMapFromMap[string, int](map[string]int{
"one": 1,
"two": 2,
"three": 3,
})

assert := internal.NewAssert(t, "TestBiMap_Get")
assert.Equal(1, biMap.Value("one"))
assert.Equal(true, biMap.ContainsKey("one"))
assert.Equal(2, biMap.Value("two"))
assert.Equal(3, biMap.Value("three"))
assert.Equal(0, biMap.Value("four"))
assert.Equal(false, biMap.ContainsKey("four"))

assert.Equal("one", biMap.Key(1))
assert.Equal(true, biMap.ContainsValue(1))
assert.Equal("two", biMap.Key(2))
assert.Equal("three", biMap.Key(3))
assert.Equal("", biMap.Key(4))
assert.Equal(false, biMap.ContainsValue(4))

assert.Equal(2, len(biMap.Keys(1, 2)))
assert.Equal(true, slices.Contains(biMap.Keys(1, 2), "one"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests do not include checking if order of returned slice corresponds to order of input slice.

The test in line 33 (Equal(2, len(biMap.Keys(1,2)) only covers the case when the given elements exist. Tests should also include what should be the behaviour when some of the input parameters are not in the BiMap -- should that issue an error (preferred), or return a subset (less optimal).

assert.Equal(false, slices.Contains(biMap.Keys(1, 2), "three"))

assert.Equal(1, len(biMap.Values("one")))
assert.Equal(true, slices.Contains(biMap.Values("one"), 1))
assert.Equal(false, slices.Contains(biMap.Values("one"), 2))

}

func TestBiMap_Contains(t *testing.T) {
biMap, _ := NewBiMapFromMap[string, int](map[string]int{
"one": 1,
"two": 2,
"three": 3,
})

assert := internal.NewAssert(t, "TestBiMap_Contains")

assert.Equal(false, biMap.ContainsKey("zero"))
assert.Equal(true, biMap.ContainsKey("one"))
assert.Equal(true, biMap.ContainsKey("two"))
assert.Equal(true, biMap.ContainsKey("three"))
assert.Equal(false, biMap.ContainsKey("four"))

assert.Equal(false, biMap.ContainsValue(0))
assert.Equal(true, biMap.ContainsValue(1))
assert.Equal(true, biMap.ContainsValue(2))
assert.Equal(true, biMap.ContainsValue(3))
assert.Equal(false, biMap.ContainsValue(4))
}

func TestBiMap_Put(t *testing.T) {
biMap := NewBiMap[string, int]()

assert := internal.NewAssert(t, "TestBiMap_Put")

err := biMap.Put("one", 1)
assert.IsNil(err)
assert.Equal(1, biMap.Len())
err = biMap.Put("one", 2)
assert.IsNotNil(err)
assert.Equal(1, biMap.Len())
err = biMap.Put("two", 1)
assert.IsNotNil(err)
err = biMap.Put("two", 2)
assert.IsNil(err)
assert.Equal(2, biMap.Len())

err = biMap.PutForce("one", 2)
assert.IsNil(err)
assert.Equal(1, biMap.Len())
assert.Equal(true, biMap.ContainsKey("one"))
assert.Equal(true, biMap.ContainsValue(2))
assert.Equal(false, biMap.ContainsKey("two"))
assert.Equal(false, biMap.ContainsValue(1))

_, err = NewBiMapFromMap(map[string]int{
"one": 1,
"two": 2,
"three": 3,
"1": 1,
})
assert.IsNotNil(err)
}

func TestBiMap_Remove(t *testing.T) {
biMap, _ := NewBiMapFromMap[string, int](map[string]int{
"one": 1,
"two": 2,
"three": 3,
"four": 4,
"five": 5,
"six": 6,
})

assert := internal.NewAssert(t, "TestBiMap_Remove")
biMap.RemoveKey("one")
assert.Equal(false, biMap.ContainsKey("one"))
assert.Equal(false, biMap.ContainsValue(1))
assert.Equal(true, biMap.ContainsKey("two"))
assert.Equal(true, biMap.ContainsValue(2))
assert.Equal(5, biMap.Len())

biMap.RemoveValue(5)
assert.Equal(false, biMap.ContainsKey("five"))
assert.Equal(false, biMap.ContainsValue(5))
assert.Equal(true, biMap.ContainsKey("four"))
assert.Equal(true, biMap.ContainsValue(6))
assert.Equal(4, biMap.Len())
assert.Equal(4, len(biMap.ToMap()))
assert.Equal(4, len(biMap.AllKeys()))
assert.Equal(4, len(biMap.AllValues()))

biMap.RemoveKey("seven")
biMap.RemoveValue(7)
assert.Equal(4, biMap.Len())

biMap.Clear()
assert.Equal(0, biMap.Len())
assert.Equal(false, biMap.ContainsKey("four"))
assert.Equal(false, biMap.ContainsValue(6))
assert.Equal(0, len(biMap.ToMap()))
assert.Equal(0, len(biMap.Keys()))
assert.Equal(0, len(biMap.Values()))
}

func TestBiMap_Inverse(t *testing.T) {
_biMap, _ := NewBiMapFromMap[string, int](map[string]int{
"one": 1,
"two": 2,
"three": 3,
})
biMap := _biMap.Inverse()

assert := internal.NewAssert(t, "TestBiMap_Inverse")
assert.Equal(true, biMap.ContainsKey(1))
assert.Equal(true, biMap.ContainsValue("one"))
assert.Equal(3, biMap.Len())
}

func TestBiMap_Json(t *testing.T) {
_biMap, _ := NewBiMapFromMap[string, int](map[string]int{
"one": 1,
"two": 2,
"three": 3,
})
assert := internal.NewAssert(t, "TestBiMap_Inverse")

j, err := json.Marshal(_biMap)
assert.IsNil(err)

biMap1 := BiMap[string, int]{}
err = json.Unmarshal(j, &biMap1)
assert.IsNil(err)
assert.Equal(1, biMap1.Value("one"))
assert.Equal("one", biMap1.Key(1))
assert.Equal(3, biMap1.Len())

biMap2 := NewBiMap[string, int]()
err = biMap2.UnmarshalJSON([]byte("{;"))
assert.IsNotNil(err)

}