From af3b7934e8f2969af242bb0e9fbf2529f3e21362 Mon Sep 17 00:00:00 2001 From: "Frederic G. MARAND" Date: Fri, 13 May 2022 23:22:27 +0200 Subject: [PATCH 1/6] feat(bst): working except delete not implemented. - TODO: interface - TODO: support sort.Comparable too - TODO: make Walk*Order interruptible by function - TODO: make IndexOf at least stop on found --- binarysearchtree/bst.go | 116 +++++++++++++++++++++++ binarysearchtree/bst_opaque_test.go | 61 ++++++++++++ binarysearchtree/bst_transparent_test.go | 83 ++++++++++++++++ go.mod | 5 +- 4 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 binarysearchtree/bst.go create mode 100644 binarysearchtree/bst_opaque_test.go create mode 100644 binarysearchtree/bst_transparent_test.go diff --git a/binarysearchtree/bst.go b/binarysearchtree/bst.go new file mode 100644 index 0000000..825cb77 --- /dev/null +++ b/binarysearchtree/bst.go @@ -0,0 +1,116 @@ +package binarysearchtree + +import ( + "golang.org/x/exp/constraints" +) + +type BST[E constraints.Ordered] struct { + data *E + left, right *BST[E] +} + +// Upsert adds a value to the tree, replacing and returning the previous one if any. +// If none existed, it returns nil. +func (t *BST[E]) Upsert(e *E) *E { + switch { + case t == nil, e == nil: + return nil + case t.data == nil: + t.data = e + return nil + case *e == *t.data: + past := t.data + t.data = e + return past + case *e > *t.data: + if t.right == nil { + t.right = &BST[E]{data: e} + return nil + } + return t.right.Upsert(e) + // The default case only covers the last "*e < *t.data" case, but if we only + // use that clause, the compiler thinks some cases are not covered. + default: + if t.left == nil { + t.left = &BST[E]{data: e} + return nil + } + return t.left.Upsert(e) + } +} + +func (t *BST[E]) Delete(e *E) { + // leaf: do it + // one child: promote it + // two children: promote the leftmost child of the right tree as the root. + // If it had a right child (can't have a left child since it is rightmost), promote it. +} + +// IndexOf returns the position of the value among those in the tree. +// If the value cannot be found, it will return 0, false, otherwise the position +// starting at 0, and true. +func (t *BST[E]) IndexOf(e *E) (int, bool) { + index, found := 0, false + t.WalkInOrder(func(x *E) { + if *x == *e { + found = true + } + if !found { + index++ + } + }) + if !found { + index = 0 + } + return index, found +} + +// WalkInOrder in useful for search and listing nodes in order. +func (t *BST[E]) WalkInOrder(fn func(e *E)) { + if t == nil { + return + } + if t.left != nil { + t.left.WalkInOrder(fn) + } + fn(t.data) + if t.right != nil { + t.right.WalkInOrder(fn) + } +} + +// WalkPostOrder in useful for deleting subtrees. +func (t *BST[E]) WalkPostOrder(fn func(e *E)) { + if t == nil { + return + } + if t.left != nil { + t.left.WalkPostOrder(fn) + } + if t.right != nil { + t.right.WalkPostOrder(fn) + } + fn(t.data) +} + +// WalkPreOrder is useful to clone the tree. +func (t *BST[E]) WalkPreOrder(fn func(e *E)) { + if t == nil { + return + } + fn(t.data) + if t.left != nil { + t.left.WalkPreOrder(fn) + } + if t.right != nil { + t.right.WalkPreOrder(fn) + } +} + +func (t *BST[E]) Clone() *BST[E] { + clone := &BST[E]{} + t.WalkPreOrder(func(e *E) { + clone.Upsert(e) + }) + return clone +} diff --git a/binarysearchtree/bst_opaque_test.go b/binarysearchtree/bst_opaque_test.go new file mode 100644 index 0000000..23e3746 --- /dev/null +++ b/binarysearchtree/bst_opaque_test.go @@ -0,0 +1,61 @@ +package binarysearchtree_test + +import ( + "strconv" + "testing" + + "github.com/fgm/container/binarysearchtree" +) + +func TestBST_nil(t *testing.T) { + var bst *binarysearchtree.BST[int] + bst.WalkInOrder(binarysearchtree.P) + bst.WalkPostOrder(binarysearchtree.P) + bst.WalkPreOrder(binarysearchtree.P) + bst.Upsert(nil) + // Output: +} + +func TestBST_Upsert(t *testing.T) { + one, six := 1, 6 + bst := binarysearchtree.Simple() + actual := bst.Upsert(&one) + if actual == nil { + t.Fatalf("expected overwriting upsert to return value, got nil") + } + if *actual != one { + t.Fatalf("expected overwriting upsert to return %d, got %d", one, *actual) + } + + actual = bst.Upsert(&six) + if actual != nil { + t.Fatalf("expected non-overwriting upsert to return nil, got %d", *actual) + } +} + +func TestBST_IndexOf(t *testing.T) { + bst := binarysearchtree.Simple() + checks := []struct { + input int + expectedOK bool + expectedIndex int + }{ + {1, true, 0}, + {2, true, 1}, + {3, true, 2}, + {4, true, 3}, + {5, true, 4}, + {6, false, 0}, + } + for _, check := range checks { + t.Run(strconv.Itoa(check.input), func(t *testing.T) { + actualIndex, actualOK := bst.IndexOf(&check.input) + if actualOK != check.expectedOK { + t.Fatalf("%d found: %t but expected %t", check.input, actualOK, check.expectedOK) + } + if actualIndex != check.expectedIndex { + t.Fatalf("%d at index %d but expected %d", check.input, actualIndex, check.expectedIndex) + } + }) + } +} diff --git a/binarysearchtree/bst_transparent_test.go b/binarysearchtree/bst_transparent_test.go new file mode 100644 index 0000000..231a631 --- /dev/null +++ b/binarysearchtree/bst_transparent_test.go @@ -0,0 +1,83 @@ +package binarysearchtree + +import ( + "fmt" + "testing" +) + +var ( + one, two, three, four, five = 1, 2, 3, 4, 5 +) + +func Simple() *BST[int] { + return &BST[int]{ + data: &three, + left: &BST[int]{data: &two, left: &BST[int]{data: &one}}, + right: &BST[int]{data: &four, right: &BST[int]{data: &five}}, + } +} + +func P(e *int) { + _, _ = fmt.Println(*e) +} + +func ExampleBST_WalkInOrder() { + bst := Simple() + bst.WalkInOrder(P) + // Output: + // 1 + // 2 + // 3 + // 4 + // 5 +} + +func ExampleBST_WalkPostOrder() { + bst := Simple() + bst.WalkPostOrder(P) + // Output: + // 1 + // 2 + // 5 + // 4 + // 3 +} + +func ExampleBST_WalkPreOrder() { + bst := Simple() + bst.WalkPreOrder(P) + // Output: + // 3 + // 2 + // 1 + // 4 + // 5 +} + +func TestBST_Clone(t *testing.T) { + bst := Simple() + clone := bst.Clone() + checks := []struct { + name string + expected, actual any + }{ + {"root", *bst.data, *clone.data}, + {"left.data", *bst.left.data, *clone.left.data}, + {"left.left.data", *bst.left.left.data, *clone.left.left.data}, + {"left.left.left", bst.left.left.left, clone.left.left.left}, + {"left.left.right", bst.left.left.right, clone.left.left.right}, + {"left.right", bst.left.right, clone.left.right}, + {"right.data", *bst.right.data, *clone.right.data}, + {"right.left", bst.right.left, clone.right.left}, + {"right.right.data", *bst.right.right.data, *clone.right.right.data}, + {"right.right.left", bst.right.right.left, clone.right.right.left}, + {"right.right.right", bst.right.right.right, clone.right.right.right}, + } + for _, check := range checks { + t.Run(check.name, func(t *testing.T) { + if check.actual != check.expected { + t.Fatalf("got %v but expected %d", check.actual, check.expected) + } + }) + } +} diff --git a/go.mod b/go.mod index 0561022..d064384 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,10 @@ module github.com/fgm/container go 1.24.7 -require github.com/google/go-cmp v0.7.0 +require ( + github.com/google/go-cmp v0.7.0 + golang.org/x/exp v0.0.0-20250305212735-054e65f0b394 +) require ( github.com/BurntSushi/toml v1.4.1-0.20240526193622-a339e1f7089c // indirect From 515eaee9e3b1267edcac0bf5e55bf9195ea2be7b Mon Sep 17 00:00:00 2001 From: "Frederic G. MARAND" Date: Sat, 14 May 2022 19:49:44 +0200 Subject: [PATCH 2/6] feat(bst): working delete. --- .idea/runConfigurations/Test_all.xml | 24 +++ binarysearchtree/bst.go | 184 ++++++++++++++++------- binarysearchtree/bst_opaque_test.go | 36 ++--- binarysearchtree/bst_transparent_test.go | 75 ++++++--- 4 files changed, 229 insertions(+), 90 deletions(-) create mode 100644 .idea/runConfigurations/Test_all.xml diff --git a/.idea/runConfigurations/Test_all.xml b/.idea/runConfigurations/Test_all.xml new file mode 100644 index 0000000..299d2a5 --- /dev/null +++ b/.idea/runConfigurations/Test_all.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/binarysearchtree/bst.go b/binarysearchtree/bst.go index 825cb77..438462f 100644 --- a/binarysearchtree/bst.go +++ b/binarysearchtree/bst.go @@ -4,44 +4,138 @@ import ( "golang.org/x/exp/constraints" ) -type BST[E constraints.Ordered] struct { +type walkCB[E constraints.Ordered] func(*E) + +type node[E constraints.Ordered] struct { data *E - left, right *BST[E] + left, right *node[E] } -// Upsert adds a value to the tree, replacing and returning the previous one if any. -// If none existed, it returns nil. -func (t *BST[E]) Upsert(e *E) *E { +func (n *node[E]) delete(e *E) *node[E] { + switch { + case n == nil, e == nil: + n = nil + case *e < *n.data: + n.left = n.left.delete(e) + case *e > *n.data: + n.right = n.right.delete(e) + // Matched childless node: just drop it. + case n.left == nil && n.right == nil: + n = nil + // Matched node with only one right child: promote that child. + case n.left == nil: + n = n.right + // Matched node with only one left child: promote that child. + case n.right == nil: + n = n.left + // Matched node with two children: promote leftmost child of right child. + // + // We could also have promoted the rightmost child of the left child. + default: + promovendum := n.right // Cannot be nil: that case was handled ed above. + for { + if promovendum.left == nil { + break + } + promovendum = promovendum.left // Not nil either, per previous statement. + } + n.data = promovendum.data + n.right = n.right.delete(promovendum.data) // As the leftmost child, it won't have two children. + } + return n +} + +func (n *node[E]) upsert(m *node[E]) *E { switch { - case t == nil, e == nil: - return nil - case t.data == nil: - t.data = e - return nil - case *e == *t.data: - past := t.data - t.data = e - return past - case *e > *t.data: - if t.right == nil { - t.right = &BST[E]{data: e} + case *m.data < *n.data: + if n.left == nil { + n.left = m return nil + } else { + return n.left.upsert(m) } - return t.right.Upsert(e) - // The default case only covers the last "*e < *t.data" case, but if we only - // use that clause, the compiler thinks some cases are not covered. - default: - if t.left == nil { - t.left = &BST[E]{data: e} + case *m.data > *n.data: + if n.right == nil { + n.right = m return nil + } else { + return n.right.upsert(m) } - return t.left.Upsert(e) + default: // *m.data == *n.data + data := n.data + n.data = m.data + return data } } -func (t *BST[E]) Delete(e *E) { - // leaf: do it - // one child: promote it +func (n *node[E]) walkInOrder(cb walkCB[E]) { + if n == nil { + return + } + if n.left != nil { + n.left.walkInOrder(cb) + } + cb(n.data) + if n.right != nil { + n.right.walkInOrder(cb) + } +} + +func (n *node[E]) walkPostOrder(cb walkCB[E]) { + if n == nil { + return + } + if n.left != nil { + n.left.walkPostOrder(cb) + } + if n.right != nil { + n.right.walkPostOrder(cb) + } + cb(n.data) +} + +func (n *node[E]) walkPreOrder(cb walkCB[E]) { + if n == nil { + return + } + cb(n.data) + if n.left != nil { + n.left.walkPreOrder(cb) + } + if n.right != nil { + n.right.walkPreOrder(cb) + } +} + +type Tree[E constraints.Ordered] struct { + root *node[E] +} + +// Upsert adds a value to the tree, replacing and returning the previous one if any. +// If none existed, it returns nil. +func (t *Tree[E]) Upsert(e ...*E) []*E { + res := make([]*E, 0, len(e)) + for _, oneE := range e { + n := &node[E]{data: oneE} + + switch { + case t == nil, e == nil: + res = append(res, nil) + case t.root == nil: + t.root = n + res = append(res, nil) + default: + res = append(res, t.root.upsert(n)) + } + } + return res +} + +func (t *Tree[E]) Delete(e *E) { + if t == nil || e == nil { + return + } + t.root.delete(e) // two children: promote the leftmost child of the right tree as the root. // If it had a right child (can't have a left child since it is rightmost), promote it. } @@ -49,7 +143,7 @@ func (t *BST[E]) Delete(e *E) { // IndexOf returns the position of the value among those in the tree. // If the value cannot be found, it will return 0, false, otherwise the position // starting at 0, and true. -func (t *BST[E]) IndexOf(e *E) (int, bool) { +func (t *Tree[E]) IndexOf(e *E) (int, bool) { index, found := 0, false t.WalkInOrder(func(x *E) { if *x == *e { @@ -66,49 +160,31 @@ func (t *BST[E]) IndexOf(e *E) (int, bool) { } // WalkInOrder in useful for search and listing nodes in order. -func (t *BST[E]) WalkInOrder(fn func(e *E)) { +func (t *Tree[E]) WalkInOrder(cb walkCB[E]) { if t == nil { return } - if t.left != nil { - t.left.WalkInOrder(fn) - } - fn(t.data) - if t.right != nil { - t.right.WalkInOrder(fn) - } + t.root.walkInOrder(cb) } // WalkPostOrder in useful for deleting subtrees. -func (t *BST[E]) WalkPostOrder(fn func(e *E)) { +func (t *Tree[E]) WalkPostOrder(fn func(e *E)) { if t == nil { return } - if t.left != nil { - t.left.WalkPostOrder(fn) - } - if t.right != nil { - t.right.WalkPostOrder(fn) - } - fn(t.data) + t.root.walkPostOrder(fn) } // WalkPreOrder is useful to clone the tree. -func (t *BST[E]) WalkPreOrder(fn func(e *E)) { +func (t *Tree[E]) WalkPreOrder(cb walkCB[E]) { if t == nil { return } - fn(t.data) - if t.left != nil { - t.left.WalkPreOrder(fn) - } - if t.right != nil { - t.right.WalkPreOrder(fn) - } + t.root.walkPreOrder(cb) } -func (t *BST[E]) Clone() *BST[E] { - clone := &BST[E]{} +func (t *Tree[E]) Clone() *Tree[E] { + clone := &Tree[E]{} t.WalkPreOrder(func(e *E) { clone.Upsert(e) }) diff --git a/binarysearchtree/bst_opaque_test.go b/binarysearchtree/bst_opaque_test.go index 23e3746..ccc5648 100644 --- a/binarysearchtree/bst_opaque_test.go +++ b/binarysearchtree/bst_opaque_test.go @@ -8,7 +8,7 @@ import ( ) func TestBST_nil(t *testing.T) { - var bst *binarysearchtree.BST[int] + var bst *binarysearchtree.Tree[int] bst.WalkInOrder(binarysearchtree.P) bst.WalkPostOrder(binarysearchtree.P) bst.WalkPreOrder(binarysearchtree.P) @@ -17,35 +17,37 @@ func TestBST_nil(t *testing.T) { } func TestBST_Upsert(t *testing.T) { - one, six := 1, 6 bst := binarysearchtree.Simple() - actual := bst.Upsert(&one) - if actual == nil { - t.Fatalf("expected overwriting upsert to return value, got nil") + actual := bst.Upsert(&binarysearchtree.One) + if len(actual) != 1 { + t.Fatalf("expected overwriting upsert to return one value, got %v", actual) } - if *actual != one { - t.Fatalf("expected overwriting upsert to return %d, got %d", one, *actual) + if *actual[0] != binarysearchtree.One { + t.Fatalf("expected overwriting upsert to return %d, got %d", binarysearchtree.One, *actual[0]) } - actual = bst.Upsert(&six) - if actual != nil { - t.Fatalf("expected non-overwriting upsert to return nil, got %d", *actual) + actual = bst.Upsert(&binarysearchtree.Six) + if len(actual) != 1 { + t.Fatalf("expected non-overwriting upsert to return one value, got %v", actual) + } + if actual[0] != nil { + t.Fatalf("expected non-overwriting upsert to return one nil, got %v", actual[0]) } } func TestBST_IndexOf(t *testing.T) { bst := binarysearchtree.Simple() - checks := []struct { + checks := [...]struct { input int expectedOK bool expectedIndex int }{ - {1, true, 0}, - {2, true, 1}, - {3, true, 2}, - {4, true, 3}, - {5, true, 4}, - {6, false, 0}, + {binarysearchtree.One, true, 0}, + {binarysearchtree.Two, true, 1}, + {binarysearchtree.Three, true, 2}, + {binarysearchtree.Four, true, 3}, + {binarysearchtree.Five, true, 4}, + {binarysearchtree.Six, false, 0}, } for _, check := range checks { t.Run(strconv.Itoa(check.input), func(t *testing.T) { diff --git a/binarysearchtree/bst_transparent_test.go b/binarysearchtree/bst_transparent_test.go index 231a631..5c1b4f8 100644 --- a/binarysearchtree/bst_transparent_test.go +++ b/binarysearchtree/bst_transparent_test.go @@ -6,15 +6,31 @@ import ( ) var ( - one, two, three, four, five = 1, 2, 3, 4, 5 + One, Two, Three, Four, Five, Six = 1, 2, 3, 4, 5, 6 ) -func Simple() *BST[int] { - return &BST[int]{ - data: &three, - left: &BST[int]{data: &two, left: &BST[int]{data: &one}}, - right: &BST[int]{data: &four, right: &BST[int]{data: &five}}, - } +// Simple builds this tree: +// 3 +// / \ +// 2 4 +// / \ +// 1 5 +func Simple() *Tree[int] { + simple := Tree[int]{} + simple.Upsert(&Three, &Two, &Four, &One, &Five) + return &simple +} + +// HalfFull builds this tree, which contains all deletion cases +// 3 +// / \ +// 2 5 +// / / \ +// 1 4 6 +func HalfFull() *Tree[int] { + hf := Tree[int]{} + hf.Upsert(&Three, &Two, &Five, &One, &Four, &Six) + return &hf } func P(e *int) { @@ -57,21 +73,23 @@ func ExampleBST_WalkPreOrder() { func TestBST_Clone(t *testing.T) { bst := Simple() clone := bst.Clone() - checks := []struct { + input := bst.root + output := clone.root + checks := [...]struct { name string expected, actual any }{ - {"root", *bst.data, *clone.data}, - {"left.data", *bst.left.data, *clone.left.data}, - {"left.left.data", *bst.left.left.data, *clone.left.left.data}, - {"left.left.left", bst.left.left.left, clone.left.left.left}, - {"left.left.right", bst.left.left.right, clone.left.left.right}, - {"left.right", bst.left.right, clone.left.right}, - {"right.data", *bst.right.data, *clone.right.data}, - {"right.left", bst.right.left, clone.right.left}, - {"right.right.data", *bst.right.right.data, *clone.right.right.data}, - {"right.right.left", bst.right.right.left, clone.right.right.left}, - {"right.right.right", bst.right.right.right, clone.right.right.right}, + {"root", *input.data, *output.data}, + {"left.data", *input.left.data, *output.left.data}, + {"left.left.data", *input.left.left.data, *output.left.left.data}, + {"left.left.left", input.left.left.left, output.left.left.left}, + {"left.left.right", input.left.left.right, output.left.left.right}, + {"left.right", input.left.right, output.left.right}, + {"right.data", *input.right.data, *output.right.data}, + {"right.left", input.right.left, output.right.left}, + {"right.right.data", *input.right.right.data, *output.right.right.data}, + {"right.right.left", input.right.right.left, output.right.right.left}, + {"right.right.right", input.right.right.right, output.right.right.right}, } for _, check := range checks { t.Run(check.name, func(t *testing.T) { @@ -81,3 +99,22 @@ func TestBST_Clone(t *testing.T) { }) } } + +func TestTree_Delete(t *testing.T) { + checks := [...]struct { + name string + delendum int + }{ + {"one: leaf", One}, + {"two: one child", Two}, + {"five: two children", Five}, + {"three: root with two children", Three}, + } + + for _, check := range checks { + t.Run(check.name, func(t *testing.T) { + bst := HalfFull() + bst.Delete(&check.delendum) + }) + } +} From 1b0f0966be8cdf25fc058f10e5147b5315817b9a Mon Sep 17 00:00:00 2001 From: "Frederic G. MARAND" Date: Sat, 14 May 2022 23:19:23 +0200 Subject: [PATCH 3/6] chore(bst): 100% test coverage. --- README.md | 2 +- binarysearchtree/bst_opaque_test.go | 63 ---------- binarysearchtree/{bst.go => intrinsic.go} | 65 +++++++---- binarysearchtree/intrinsic_opaque_test.go | 110 ++++++++++++++++++ ..._test.go => intrinsic_transparent_test.go} | 14 ++- go.mod | 5 +- types.go | 26 ++++- 7 files changed, 185 insertions(+), 100 deletions(-) delete mode 100644 binarysearchtree/bst_opaque_test.go rename binarysearchtree/{bst.go => intrinsic.go} (68%) create mode 100644 binarysearchtree/intrinsic_opaque_test.go rename binarysearchtree/{bst_transparent_test.go => intrinsic_transparent_test.go} (89%) diff --git a/README.md b/README.md index 546462b..7b79915 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ Since this is a library, it has no install process, but you can build it to ensu #### Running normal tests: unit and benchmarks -For the simple version, without generating coverage reports, run: +For the simple version, run: ``` make test # Unit tests, fast make coverage # Coverage report in cover.out. A bit longer. diff --git a/binarysearchtree/bst_opaque_test.go b/binarysearchtree/bst_opaque_test.go deleted file mode 100644 index ccc5648..0000000 --- a/binarysearchtree/bst_opaque_test.go +++ /dev/null @@ -1,63 +0,0 @@ -package binarysearchtree_test - -import ( - "strconv" - "testing" - - "github.com/fgm/container/binarysearchtree" -) - -func TestBST_nil(t *testing.T) { - var bst *binarysearchtree.Tree[int] - bst.WalkInOrder(binarysearchtree.P) - bst.WalkPostOrder(binarysearchtree.P) - bst.WalkPreOrder(binarysearchtree.P) - bst.Upsert(nil) - // Output: -} - -func TestBST_Upsert(t *testing.T) { - bst := binarysearchtree.Simple() - actual := bst.Upsert(&binarysearchtree.One) - if len(actual) != 1 { - t.Fatalf("expected overwriting upsert to return one value, got %v", actual) - } - if *actual[0] != binarysearchtree.One { - t.Fatalf("expected overwriting upsert to return %d, got %d", binarysearchtree.One, *actual[0]) - } - - actual = bst.Upsert(&binarysearchtree.Six) - if len(actual) != 1 { - t.Fatalf("expected non-overwriting upsert to return one value, got %v", actual) - } - if actual[0] != nil { - t.Fatalf("expected non-overwriting upsert to return one nil, got %v", actual[0]) - } -} - -func TestBST_IndexOf(t *testing.T) { - bst := binarysearchtree.Simple() - checks := [...]struct { - input int - expectedOK bool - expectedIndex int - }{ - {binarysearchtree.One, true, 0}, - {binarysearchtree.Two, true, 1}, - {binarysearchtree.Three, true, 2}, - {binarysearchtree.Four, true, 3}, - {binarysearchtree.Five, true, 4}, - {binarysearchtree.Six, false, 0}, - } - for _, check := range checks { - t.Run(strconv.Itoa(check.input), func(t *testing.T) { - actualIndex, actualOK := bst.IndexOf(&check.input) - if actualOK != check.expectedOK { - t.Fatalf("%d found: %t but expected %t", check.input, actualOK, check.expectedOK) - } - if actualIndex != check.expectedIndex { - t.Fatalf("%d at index %d but expected %d", check.input, actualIndex, check.expectedIndex) - } - }) - } -} diff --git a/binarysearchtree/bst.go b/binarysearchtree/intrinsic.go similarity index 68% rename from binarysearchtree/bst.go rename to binarysearchtree/intrinsic.go index 438462f..e5dd860 100644 --- a/binarysearchtree/bst.go +++ b/binarysearchtree/intrinsic.go @@ -1,12 +1,12 @@ package binarysearchtree import ( - "golang.org/x/exp/constraints" -) + "cmp" -type walkCB[E constraints.Ordered] func(*E) + "github.com/fgm/container" +) -type node[E constraints.Ordered] struct { +type node[E cmp.Ordered] struct { data *E left, right *node[E] } @@ -68,7 +68,7 @@ func (n *node[E]) upsert(m *node[E]) *E { } } -func (n *node[E]) walkInOrder(cb walkCB[E]) { +func (n *node[E]) walkInOrder(cb container.WalkCB[E]) { if n == nil { return } @@ -81,7 +81,7 @@ func (n *node[E]) walkInOrder(cb walkCB[E]) { } } -func (n *node[E]) walkPostOrder(cb walkCB[E]) { +func (n *node[E]) walkPostOrder(cb container.WalkCB[E]) { if n == nil { return } @@ -94,7 +94,7 @@ func (n *node[E]) walkPostOrder(cb walkCB[E]) { cb(n.data) } -func (n *node[E]) walkPreOrder(cb walkCB[E]) { +func (n *node[E]) walkPreOrder(cb container.WalkCB[E]) { if n == nil { return } @@ -107,43 +107,58 @@ func (n *node[E]) walkPreOrder(cb walkCB[E]) { } } -type Tree[E constraints.Ordered] struct { +// Intrinsic holds nodes which are their own ordering key. +type Intrinsic[E cmp.Ordered] struct { root *node[E] } +// Len returns the number of nodes in the tree, for the container.Countable interface. +// Complexity is O(n). +func (t *Intrinsic[E]) Len() int { + l := 0 + t.WalkPostOrder(func(_ *E) { l++ }) + return l +} + +func (t *Intrinsic[E]) Elements() []E { + var sl []E + t.WalkPreOrder(func(e *E) { sl = append(sl, *e) }) + return sl +} + // Upsert adds a value to the tree, replacing and returning the previous one if any. // If none existed, it returns nil. -func (t *Tree[E]) Upsert(e ...*E) []*E { - res := make([]*E, 0, len(e)) +func (t *Intrinsic[E]) Upsert(e ...*E) []*E { + results := make([]*E, 0, len(e)) + var result *E for _, oneE := range e { n := &node[E]{data: oneE} switch { case t == nil, e == nil: - res = append(res, nil) + result = nil case t.root == nil: t.root = n - res = append(res, nil) + result = nil default: - res = append(res, t.root.upsert(n)) + result = t.root.upsert(n) } + results = append(results, result) } - return res + return results } -func (t *Tree[E]) Delete(e *E) { +func (t *Intrinsic[E]) Delete(e *E) { if t == nil || e == nil { return } t.root.delete(e) - // two children: promote the leftmost child of the right tree as the root. - // If it had a right child (can't have a left child since it is rightmost), promote it. } // IndexOf returns the position of the value among those in the tree. // If the value cannot be found, it will return 0, false, otherwise the position // starting at 0, and true. -func (t *Tree[E]) IndexOf(e *E) (int, bool) { +func (t *Intrinsic[E]) IndexOf(e *E) (int, bool) { index, found := 0, false t.WalkInOrder(func(x *E) { if *x == *e { @@ -159,8 +174,8 @@ func (t *Tree[E]) IndexOf(e *E) (int, bool) { return index, found } -// WalkInOrder in useful for search and listing nodes in order. -func (t *Tree[E]) WalkInOrder(cb walkCB[E]) { +// WalkInOrder is useful for search and listing nodes in order. +func (t *Intrinsic[E]) WalkInOrder(cb container.WalkCB[E]) { if t == nil { return } @@ -168,23 +183,23 @@ func (t *Tree[E]) WalkInOrder(cb walkCB[E]) { } // WalkPostOrder in useful for deleting subtrees. -func (t *Tree[E]) WalkPostOrder(fn func(e *E)) { +func (t *Intrinsic[E]) WalkPostOrder(cb container.WalkCB[E]) { if t == nil { return } - t.root.walkPostOrder(fn) + t.root.walkPostOrder(cb) } // WalkPreOrder is useful to clone the tree. -func (t *Tree[E]) WalkPreOrder(cb walkCB[E]) { +func (t *Intrinsic[E]) WalkPreOrder(cb container.WalkCB[E]) { if t == nil { return } t.root.walkPreOrder(cb) } -func (t *Tree[E]) Clone() *Tree[E] { - clone := &Tree[E]{} +func (t *Intrinsic[E]) Clone() container.BinarySearchTree[E] { + clone := &Intrinsic[E]{} t.WalkPreOrder(func(e *E) { clone.Upsert(e) }) diff --git a/binarysearchtree/intrinsic_opaque_test.go b/binarysearchtree/intrinsic_opaque_test.go new file mode 100644 index 0000000..6290268 --- /dev/null +++ b/binarysearchtree/intrinsic_opaque_test.go @@ -0,0 +1,110 @@ +package binarysearchtree_test + +import ( + "strconv" + "testing" + + "github.com/fgm/container" + bst "github.com/fgm/container/binarysearchtree" +) + +func TestIntrinsic_nil(t *testing.T) { + var tree *bst.Intrinsic[int] + tree.WalkInOrder(bst.P) + tree.WalkPostOrder(bst.P) + tree.WalkPreOrder(bst.P) + tree.Upsert(nil) + tree.Delete(nil) + + tree = &bst.Intrinsic[int]{} + tree.WalkInOrder(bst.P) + tree.WalkPostOrder(bst.P) + tree.WalkPreOrder(bst.P) + // Output: +} + +func TestIntrinsic_Upsert(t *testing.T) { + tree := bst.Simple() + actual := tree.Upsert(&bst.One) + if len(actual) != 1 { + t.Fatalf("expected overwriting upsert to return one value, got %v", actual) + } + if *actual[0] != bst.One { + t.Fatalf("expected overwriting upsert to return %d, got %d", bst.One, *actual[0]) + } + + actual = tree.Upsert(&bst.Six) + if len(actual) != 1 { + t.Fatalf("expected non-overwriting upsert to return one value, got %v", actual) + } + if actual[0] != nil { + t.Fatalf("expected non-overwriting upsert to return one nil, got %v", actual[0]) + } +} + +func TestIntrinsic_IndexOf(t *testing.T) { + tree := bst.Simple() + checks := [...]struct { + input int + expectedOK bool + expectedIndex int + }{ + {bst.One, true, 0}, + {bst.Two, true, 1}, + {bst.Three, true, 2}, + {bst.Four, true, 3}, + {bst.Five, true, 4}, + {bst.Six, false, 0}, + } + for _, check := range checks { + t.Run(strconv.Itoa(check.input), func(t *testing.T) { + actualIndex, actualOK := tree.IndexOf(&check.input) + if actualOK != check.expectedOK { + t.Fatalf("%d found: %t but expected %t", check.input, actualOK, check.expectedOK) + } + if actualIndex != check.expectedIndex { + t.Fatalf("%d at index %d but expected %d", check.input, actualIndex, check.expectedIndex) + } + }) + } +} + +func TestIntrinsic_Len(t *testing.T) { + si := bst.Simple().(container.Enumerable[int]).Elements() + hf := bst.HalfFull().(container.Enumerable[int]).Elements() + + checks := [...]struct { + name string + input []int + deletions []int + expected int + }{ + {"empty", nil, nil, 0}, + {"simple", si, nil, 5}, + {"half-full", hf, nil, 6}, + {"overwrite element", append(si, bst.Three), nil, 5}, + {"delete nonexistent", si, []int{bst.Six}, 5}, + {"delete existing childless", si, []int{bst.One}, 4}, + {"delete existing with 1 left child", si, []int{bst.Two}, 4}, + {"delete existing with 1 right child", si, []int{bst.Four}, 4}, + {"delete existing with 2 children", hf, []int{bst.Three}, 5}, + } + for _, check := range checks { + t.Run(check.name, func(t *testing.T) { + tree := bst.Intrinsic[int]{} + // In these loops, e is always the same variable: without cloning, + // each iteration reuses the same pointer, overwriting the tree. + for _, e := range check.input { + clone := e + tree.Upsert(&clone) + } + for _, e := range check.deletions { + clone := e + tree.Delete(&clone) + } + if tree.Len() != check.expected { + t.Fatalf("Found len %d, but expected %d", tree.Len(), check.expected) + } + }) + } +} diff --git a/binarysearchtree/bst_transparent_test.go b/binarysearchtree/intrinsic_transparent_test.go similarity index 89% rename from binarysearchtree/bst_transparent_test.go rename to binarysearchtree/intrinsic_transparent_test.go index 5c1b4f8..e64b3f7 100644 --- a/binarysearchtree/bst_transparent_test.go +++ b/binarysearchtree/intrinsic_transparent_test.go @@ -3,6 +3,8 @@ package binarysearchtree import ( "fmt" "testing" + + "github.com/fgm/container" ) var ( @@ -15,8 +17,8 @@ var ( // 2 4 // / \ // 1 5 -func Simple() *Tree[int] { - simple := Tree[int]{} +func Simple() container.BinarySearchTree[int] { + simple := Intrinsic[int]{} simple.Upsert(&Three, &Two, &Four, &One, &Five) return &simple } @@ -27,8 +29,8 @@ func Simple() *Tree[int] { // 2 5 // / / \ // 1 4 6 -func HalfFull() *Tree[int] { - hf := Tree[int]{} +func HalfFull() container.BinarySearchTree[int] { + hf := Intrinsic[int]{} hf.Upsert(&Three, &Two, &Five, &One, &Four, &Six) return &hf } @@ -71,8 +73,8 @@ func ExampleBST_WalkPreOrder() { } func TestBST_Clone(t *testing.T) { - bst := Simple() - clone := bst.Clone() + bst := Simple().(*Intrinsic[int]) + clone := bst.Clone().(*Intrinsic[int]) input := bst.root output := clone.root checks := [...]struct { diff --git a/go.mod b/go.mod index d064384..0561022 100644 --- a/go.mod +++ b/go.mod @@ -2,10 +2,7 @@ module github.com/fgm/container go 1.24.7 -require ( - github.com/google/go-cmp v0.7.0 - golang.org/x/exp v0.0.0-20250305212735-054e65f0b394 -) +require github.com/google/go-cmp v0.7.0 require ( github.com/BurntSushi/toml v1.4.1-0.20240526193622-a339e1f7089c // indirect diff --git a/types.go b/types.go index 59ac71b..8300c4f 100644 --- a/types.go +++ b/types.go @@ -1,6 +1,9 @@ package container -import "iter" +import ( + "cmp" + "iter" +) // OrderedMap has the same API as a sync.Map for the specific case of OrderedMap[any, any]. type OrderedMap[K comparable, V any] interface { @@ -73,6 +76,8 @@ type Stack[E any] interface { // For concurrency-safe types, it is not atomic vs other operations, // meaning it MUST NOT be used to take decisions, but only as an observability/debugging tool. type Countable interface { + // Len returns the number of elements in a structure. + // Its complexity may be higher than O(1), e.g. O(n) when it relies on Enumerable. Len() int } @@ -87,3 +92,22 @@ type Set[E comparable] interface { Intersection(other Set[E]) Set[E] Difference(other Set[E]) Set[E] } + +// FIXME replace by an iterator-based version like the one in Set.Items. +type Enumerable[E any] interface { + Elements() []E +} + +// BinarySearchTree is a generic binary search tree implementation with no concurrency guarantees. +// Instantiate by a zero value of the implementation. +type BinarySearchTree[E cmp.Ordered] interface { + Clone() BinarySearchTree[E] + Delete(*E) + IndexOf(*E) (int, bool) + Upsert(...*E) []*E + WalkInOrder(cb WalkCB[E]) + WalkPostOrder(cb WalkCB[E]) + WalkPreOrder(cb WalkCB[E]) +} + +type WalkCB[E any] func(*E) From bdf17632d927b7e07102268d432261c7b0e2623e Mon Sep 17 00:00:00 2001 From: "Frederic G. MARAND" Date: Sat, 14 May 2022 23:52:08 +0200 Subject: [PATCH 4/6] Test Codecov without token. --- .github/workflows/workflow.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 01ab831..f9d6a65 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -33,7 +33,6 @@ jobs: - name: Upload coverage to Codecov uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1 with: - token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos files: ./coverage/cover.out flags: unittests name: codecov-umbrella From 294e73e75b6905a233f79a522dc1e31e7d849fb7 Mon Sep 17 00:00:00 2001 From: "Frederic G. MARAND" Date: Sun, 15 May 2022 10:41:19 +0200 Subject: [PATCH 5/6] chore(bst): 100% coverage with cancelable walks, IndexOf. --- binarysearchtree/intrinsic.go | 95 ++++++++++++------- binarysearchtree/intrinsic_opaque_test.go | 46 +++++++++ .../intrinsic_transparent_test.go | 35 +++---- types.go | 8 +- 4 files changed, 128 insertions(+), 56 deletions(-) diff --git a/binarysearchtree/intrinsic.go b/binarysearchtree/intrinsic.go index e5dd860..9189acf 100644 --- a/binarysearchtree/intrinsic.go +++ b/binarysearchtree/intrinsic.go @@ -2,6 +2,7 @@ package binarysearchtree import ( "cmp" + "errors" "github.com/fgm/container" ) @@ -68,43 +69,64 @@ func (n *node[E]) upsert(m *node[E]) *E { } } -func (n *node[E]) walkInOrder(cb container.WalkCB[E]) { +func (n *node[E]) walkInOrder(cb container.WalkCB[E]) error { + var err error if n == nil { - return + return nil } if n.left != nil { - n.left.walkInOrder(cb) + if err = n.left.walkInOrder(cb); err != nil { + return err + } + } + if err := cb(n.data); err != nil { + return err } - cb(n.data) if n.right != nil { - n.right.walkInOrder(cb) + if err = n.right.walkInOrder(cb); err != nil { + return err + } } + return nil } -func (n *node[E]) walkPostOrder(cb container.WalkCB[E]) { +func (n *node[E]) walkPostOrder(cb container.WalkCB[E]) error { + var err error if n == nil { - return + return nil } if n.left != nil { - n.left.walkPostOrder(cb) + if err = n.left.walkPostOrder(cb); err != nil { + return err + } } if n.right != nil { - n.right.walkPostOrder(cb) + if err = n.right.walkPostOrder(cb); err != nil { + return err + } } - cb(n.data) + return cb(n.data) } -func (n *node[E]) walkPreOrder(cb container.WalkCB[E]) { +func (n *node[E]) walkPreOrder(cb container.WalkCB[E]) error { + var err error if n == nil { - return + return nil + } + if err := cb(n.data); err != nil { + return err } - cb(n.data) if n.left != nil { - n.left.walkPreOrder(cb) + if err = n.left.walkPreOrder(cb); err != nil { + return err + } } if n.right != nil { - n.right.walkPreOrder(cb) + if err = n.right.walkPreOrder(cb); err != nil { + return err + } } + return nil } // Intrinsic holds nodes which are their own ordering key. @@ -116,13 +138,13 @@ type Intrinsic[E cmp.Ordered] struct { // Complexity is O(n). func (t *Intrinsic[E]) Len() int { l := 0 - t.WalkPostOrder(func(_ *E) { l++ }) + t.WalkPostOrder(func(_ *E) error { l++; return nil }) return l } func (t *Intrinsic[E]) Elements() []E { var sl []E - t.WalkPreOrder(func(e *E) { sl = append(sl, *e) }) + t.WalkPreOrder(func(e *E) error { sl = append(sl, *e); return nil }) return sl } @@ -159,49 +181,50 @@ func (t *Intrinsic[E]) Delete(e *E) { // If the value cannot be found, it will return 0, false, otherwise the position // starting at 0, and true. func (t *Intrinsic[E]) IndexOf(e *E) (int, bool) { - index, found := 0, false - t.WalkInOrder(func(x *E) { + errFound := errors.New("found") + index := 0 + err := t.WalkInOrder(func(x *E) error { if *x == *e { - found = true - } - if !found { - index++ + return errFound } + index++ + return nil }) - if !found { - index = 0 + if err != errFound { + return 0, false } - return index, found + return index, true } // WalkInOrder is useful for search and listing nodes in order. -func (t *Intrinsic[E]) WalkInOrder(cb container.WalkCB[E]) { +func (t *Intrinsic[E]) WalkInOrder(cb container.WalkCB[E]) error { if t == nil { - return + return nil } - t.root.walkInOrder(cb) + return t.root.walkInOrder(cb) } // WalkPostOrder in useful for deleting subtrees. -func (t *Intrinsic[E]) WalkPostOrder(cb container.WalkCB[E]) { +func (t *Intrinsic[E]) WalkPostOrder(cb container.WalkCB[E]) error { if t == nil { - return + return nil } - t.root.walkPostOrder(cb) + return t.root.walkPostOrder(cb) } // WalkPreOrder is useful to clone the tree. -func (t *Intrinsic[E]) WalkPreOrder(cb container.WalkCB[E]) { +func (t *Intrinsic[E]) WalkPreOrder(cb container.WalkCB[E]) error { if t == nil { - return + return nil } - t.root.walkPreOrder(cb) + return t.root.walkPreOrder(cb) } func (t *Intrinsic[E]) Clone() container.BinarySearchTree[E] { clone := &Intrinsic[E]{} - t.WalkPreOrder(func(e *E) { + t.WalkPreOrder(func(e *E) error { clone.Upsert(e) + return nil }) return clone } diff --git a/binarysearchtree/intrinsic_opaque_test.go b/binarysearchtree/intrinsic_opaque_test.go index 6290268..723e725 100644 --- a/binarysearchtree/intrinsic_opaque_test.go +++ b/binarysearchtree/intrinsic_opaque_test.go @@ -1,6 +1,8 @@ package binarysearchtree_test import ( + "fmt" + "log" "strconv" "testing" @@ -108,3 +110,47 @@ func TestIntrinsic_Len(t *testing.T) { }) } } + +func TestIntrinsic_Walk_canceling(t *testing.T) { + tree := bst.Simple() + checks := [...]struct { + name string + walker func(cb container.WalkCB[int]) error + calls1, calls3, calls5 int + }{ + {"in order", tree.WalkInOrder, 1, 3, 5}, + {"post order", tree.WalkPostOrder, 1, 5, 3}, + {"pre order", tree.WalkPreOrder, 3, 1, 5}, + } + tree.WalkInOrder(func(e *int) error { log.Println(*e); return nil }) + stopper := func(at int) container.WalkCB[int] { + called := 0 + return container.WalkCB[int](func(e *int) error { + called++ + if *e == at { + return fmt.Errorf("%d", called) + } + return nil + }) + } + for _, check := range checks { + t.Run(check.name, func(t *testing.T) { + for _, val := range []struct { + input, expected int + }{ + {1, check.calls1}, + {3, check.calls3}, + {5, check.calls5}, + } { + err := check.walker(stopper(val.input)) + actual, err := strconv.Atoi(err.Error()) + if err != nil { + t.Fatalf("unexpected non-int error: %v", err) + } + if actual != val.expected { + t.Fatalf("got %d but expected %d", actual, val.expected) + } + } + }) + } +} diff --git a/binarysearchtree/intrinsic_transparent_test.go b/binarysearchtree/intrinsic_transparent_test.go index e64b3f7..21b92a5 100644 --- a/binarysearchtree/intrinsic_transparent_test.go +++ b/binarysearchtree/intrinsic_transparent_test.go @@ -12,11 +12,12 @@ var ( ) // Simple builds this tree: -// 3 -// / \ -// 2 4 -// / \ -// 1 5 +// +// 3 +// / \ +// 2 4 +// / \ +// 1 5 func Simple() container.BinarySearchTree[int] { simple := Intrinsic[int]{} simple.Upsert(&Three, &Two, &Four, &One, &Five) @@ -24,22 +25,24 @@ func Simple() container.BinarySearchTree[int] { } // HalfFull builds this tree, which contains all deletion cases -// 3 -// / \ -// 2 5 -// / / \ -// 1 4 6 +// +// 3 +// / \ +// 2 5 +// / / \ +// 1 4 6 func HalfFull() container.BinarySearchTree[int] { hf := Intrinsic[int]{} hf.Upsert(&Three, &Two, &Five, &One, &Four, &Six) return &hf } -func P(e *int) { - _, _ = fmt.Println(*e) +func P(e *int) error { + _, err := fmt.Println(*e) + return err } -func ExampleBST_WalkInOrder() { +func ExampleIntrinsic_WalkInOrder() { bst := Simple() bst.WalkInOrder(P) // Output: @@ -50,7 +53,7 @@ func ExampleBST_WalkInOrder() { // 5 } -func ExampleBST_WalkPostOrder() { +func ExampleIntrinsic_WalkPostOrder() { bst := Simple() bst.WalkPostOrder(P) // Output: @@ -61,7 +64,7 @@ func ExampleBST_WalkPostOrder() { // 3 } -func ExampleBST_WalkPreOrder() { +func ExampleIntrinsic_WalkPreOrder() { bst := Simple() bst.WalkPreOrder(P) // Output: @@ -72,7 +75,7 @@ func ExampleBST_WalkPreOrder() { // 5 } -func TestBST_Clone(t *testing.T) { +func TestIntrinsic_Clone(t *testing.T) { bst := Simple().(*Intrinsic[int]) clone := bst.Clone().(*Intrinsic[int]) input := bst.root diff --git a/types.go b/types.go index 8300c4f..a7673c7 100644 --- a/types.go +++ b/types.go @@ -105,9 +105,9 @@ type BinarySearchTree[E cmp.Ordered] interface { Delete(*E) IndexOf(*E) (int, bool) Upsert(...*E) []*E - WalkInOrder(cb WalkCB[E]) - WalkPostOrder(cb WalkCB[E]) - WalkPreOrder(cb WalkCB[E]) + WalkInOrder(cb WalkCB[E]) error + WalkPostOrder(cb WalkCB[E]) error + WalkPreOrder(cb WalkCB[E]) error } -type WalkCB[E any] func(*E) +type WalkCB[E any] func(*E) error From b311915d0329e39c82ecd6f73b2277e784569421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20G=2E=20MARAND?= Date: Tue, 30 Sep 2025 21:24:31 +0100 Subject: [PATCH 6/6] Harden build and update it less monthly, not weekly. --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 321561f..a80eb2e 100644 --- a/Makefile +++ b/Makefile @@ -20,6 +20,7 @@ bench: cover: # This runs the benchmarks just once, as unit tests, for coverage reporting only. # It does not replace running "make bench". + mkdir -p coverage go test -v -race -run=. -bench=. -benchtime=1x -coverprofile=coverage/cover.out -covermode=atomic ./... .PHONY: test