Skip to content

Commit

Permalink
Use a pointer receiver for internal/set.Set
Browse files Browse the repository at this point in the history
... to make sure that, and clear to callers that, Add() and Delete()
modify the original object and not a copy.

Luckily(?), this has worked fine, because Set contains a map[],
which is a reference type, so a copy of Set just creates another
reference to the same map.

Eitehr way, fix the method receivers, and add tests to ensure
the code works as expected.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed May 10, 2023
1 parent 3574153 commit bc29272
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
4 changes: 2 additions & 2 deletions internal/set/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ func NewWithValues[E comparable](values ...E) *Set[E] {
return s
}

func (s Set[E]) Add(v E) {
func (s *Set[E]) Add(v E) {
s.m[v] = struct{}{} // Possibly writing the same struct{}{} presence marker again.
}

func (s Set[E]) Delete(v E) {
func (s *Set[E]) Delete(v E) {
delete(s.m, v)
}

Expand Down
66 changes: 66 additions & 0 deletions internal/set/set_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package set

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestNew(t *testing.T) {
s := New[int]()
assert.True(t, s.Empty())
}

func TestNewWithValues(t *testing.T) {
s := NewWithValues(1, 3)
assert.True(t, s.Contains(1))
assert.False(t, s.Contains(2))
assert.True(t, s.Contains(3))
}

func TestAdd(t *testing.T) {
s := NewWithValues(1)
assert.False(t, s.Contains(2))
s.Add(2)
assert.True(t, s.Contains(2))
s.Add(2) // Adding an already-present element
assert.True(t, s.Contains(2))
// Unrelated elements are unaffected
assert.True(t, s.Contains(1))
assert.False(t, s.Contains(3))
}

func TestDelete(t *testing.T) {
s := NewWithValues(1, 2)
assert.True(t, s.Contains(2))
s.Delete(2)
assert.False(t, s.Contains(2))
s.Delete(2) // Deleting a missing element
assert.False(t, s.Contains(2))
// Unrelated elements are unaffected
assert.True(t, s.Contains(1))
}

func TestContains(t *testing.T) {
s := NewWithValues(1, 2)
assert.True(t, s.Contains(1))
assert.True(t, s.Contains(2))
assert.False(t, s.Contains(3))
}

func TestEmpty(t *testing.T) {
s := New[int]()
assert.True(t, s.Empty())
s.Add(1)
assert.False(t, s.Empty())
s.Delete(1)
assert.True(t, s.Empty())
}

func TestValues(t *testing.T) {
s := New[int]()
assert.Empty(t, s.Values())
s.Add(1)
s.Add(2)
assert.ElementsMatch(t, []int{1, 2}, s.Values())
}

0 comments on commit bc29272

Please sign in to comment.