Skip to content

Commit

Permalink
fix(p/demo/avl): correct order for IterateByOffset (#3393)
Browse files Browse the repository at this point in the history 
the tests were there, but apparently `slicesEqual` had a bug. lol.

This comes from #3324, and makes it so that IterateByOffset and
ReverseIterateByOffset use the correct order. I changed the `descending`
argument of the underlying TraverseByOffset to match it with
avl.Node.TraverseInRange.

---------

Signed-off-by: moul <[email protected]>
Co-authored-by: moul <[email protected]>
  • Loading branch information
@thehowl @moul
thehowl and moul authored Dec 20, 2024
1 parent ea598bc commit 2e332dd
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 36 deletions.
20 changes: 11 additions & 9 deletions examples/gno.land/p/demo/avl/node.gno
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func (node *Node) TraverseInRange(start, end string, ascending bool, leavesOnly

// TraverseByOffset traverses all nodes, including inner nodes.
// A limit of math.MaxInt means no limit.
func (node *Node) TraverseByOffset(offset, limit int, descending bool, leavesOnly bool, cb func(*Node) bool) bool {
func (node *Node) TraverseByOffset(offset, limit int, ascending bool, leavesOnly bool, cb func(*Node) bool) bool {
if node == nil {
return false
}
Expand All @@ -401,32 +401,34 @@ func (node *Node) TraverseByOffset(offset, limit int, descending bool, leavesOnl
}

// go to the actual recursive function.
return node.traverseByOffset(offset, limit, descending, leavesOnly, cb)
return node.traverseByOffset(offset, limit, ascending, leavesOnly, cb)
}

// TraverseByOffset traverses the subtree rooted at the node by offset and limit,
// in either ascending or descending order, and applies the callback function to each traversed node.
// If leavesOnly is true, only leaf nodes are visited.
func (node *Node) traverseByOffset(offset, limit int, descending bool, leavesOnly bool, cb func(*Node) bool) bool {
func (node *Node) traverseByOffset(offset, limit int, ascending bool, leavesOnly bool, cb func(*Node) bool) bool {
// caller guarantees: offset < node.size; limit > 0.
if !leavesOnly {
if cb(node) {
return true
return true // Stop traversal if callback returns true
}
}
first, second := node.getLeftNode(), node.getRightNode()
if descending {
if !ascending {
first, second = second, first
}
if first.IsLeaf() {
// either run or skip, based on offset
if offset > 0 {
offset--
} else {
cb(first)
if cb(first) {
return true // Stop traversal if callback returns true
}
limit--
if limit <= 0 {
return false
return true // Stop traversal when limit is reached
}
}
} else {
Expand All @@ -437,7 +439,7 @@ func (node *Node) traverseByOffset(offset, limit int, descending bool, leavesOnl
if offset >= first.size {
offset -= first.size // 1
} else {
if first.traverseByOffset(offset, limit, descending, leavesOnly, cb) {
if first.traverseByOffset(offset, limit, ascending, leavesOnly, cb) {
return true
}
// number of leaves which could actually be called from inside
Expand All @@ -460,7 +462,7 @@ func (node *Node) traverseByOffset(offset, limit int, descending bool, leavesOnl
}
// => if it is not a leaf, it will still be enough to recursively call this
// function with the updated offset and limit
return second.traverseByOffset(offset, limit, descending, leavesOnly, cb)
return second.traverseByOffset(offset, limit, ascending, leavesOnly, cb)
}

// Only used in testing...
Expand Down
64 changes: 43 additions & 21 deletions examples/gno.land/p/demo/avl/node_test.gno
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,34 @@ Book
Browser`
tt := []struct {
name string
desc bool
asc bool
}{
{"ascending", false},
{"descending", true},
{"ascending", true},
{"descending", false},
}

for _, tt := range tt {
t.Run(tt.name, func(t *testing.T) {
// use sl to insert the values, and reversed to match the values
// we do this to ensure that the order of TraverseByOffset is independent
// from the insertion order
sl := strings.Split(testStrings, "\n")

// sort a first time in the order opposite to how we'll be traversing
// the tree, to ensure that we are not just iterating through with
// insertion order.
sort.Strings(sl)
if !tt.desc {
reverseSlice(sl)
reversed := append([]string{}, sl...)
reverseSlice(reversed)

if !tt.asc {
sl, reversed = reversed, sl
}

r := NewNode(sl[0], nil)
for _, v := range sl[1:] {
r := NewNode(reversed[0], nil)
for _, v := range reversed[1:] {
r, _ = r.Set(v, nil)
}

// then sort sl in the order we'll be traversing it, so that we can
// compare the result with sl.
reverseSlice(sl)

var result []string
for i := 0; i < len(sl); i++ {
r.TraverseByOffset(i, 1, tt.desc, true, func(n *Node) bool {
r.TraverseByOffset(i, 1, tt.asc, true, func(n *Node) bool {
result = append(result, n.Key())
return false
})
Expand All @@ -66,7 +64,7 @@ Browser`
exp := sl[i:max]
actual := []string{}

r.TraverseByOffset(i, l, tt.desc, true, func(tr *Node) bool {
r.TraverseByOffset(i, l, tt.asc, true, func(tr *Node) bool {
actual = append(actual, tr.Key())
return false
})
Expand Down Expand Up @@ -422,6 +420,30 @@ func TestTraverse(t *testing.T) {
t.Errorf("want %v got %v", expected, result)
}
})

t.Run("early termination", func(t *testing.T) {
if len(tt.input) == 0 {
return // Skip for empty tree
}

var result []string
var count int
tree.Iterate("", "", func(n *Node) bool {
count++
result = append(result, n.Key())
return true // Stop after first item
})

if count != 1 {
t.Errorf("Expected callback to be called exactly once, got %d calls", count)
}
if len(result) != 1 {
t.Errorf("Expected exactly one result, got %d items", len(result))
}
if len(result) > 0 && result[0] != tt.expected[0] {
t.Errorf("Expected first item to be %v, got %v", tt.expected[0], result[0])
}
})
})
}
}
Expand All @@ -435,7 +457,7 @@ func TestRotateWhenHeightDiffers(t *testing.T) {
{
"right rotation when left subtree is higher",
[]string{"E", "C", "A", "B", "D"},
[]string{"A", "B", "C", "E", "D"},
[]string{"A", "B", "C", "D", "E"},
},
{
"left rotation when right subtree is higher",
Expand All @@ -445,12 +467,12 @@ func TestRotateWhenHeightDiffers(t *testing.T) {
{
"left-right rotation",
[]string{"E", "A", "C", "B", "D"},
[]string{"A", "B", "C", "E", "D"},
[]string{"A", "B", "C", "D", "E"},
},
{
"right-left rotation",
[]string{"A", "E", "C", "B", "D"},
[]string{"A", "B", "C", "E", "D"},
[]string{"A", "B", "C", "D", "E"},
},
}

Expand Down Expand Up @@ -533,7 +555,7 @@ func slicesEqual(w1, w2 []string) bool {
return false
}
for i := 0; i < len(w1); i++ {
if w1[0] != w2[0] {
if w1[i] != w2[i] {
return false
}
}
Expand Down
4 changes: 2 additions & 2 deletions examples/gno.land/p/demo/avl/pager/pager.gno
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ func (p *Pager) GetPageWithSize(pageNumber, pageSize int) *Page {
items := []Item{}

if p.Reversed {
p.Tree.IterateByOffset(startIndex, endIndex-startIndex, func(key string, value interface{}) bool {
p.Tree.ReverseIterateByOffset(startIndex, endIndex-startIndex, func(key string, value interface{}) bool {
items = append(items, Item{Key: key, Value: value})
return false
})
} else {
p.Tree.ReverseIterateByOffset(startIndex, endIndex-startIndex, func(key string, value interface{}) bool {
p.Tree.IterateByOffset(startIndex, endIndex-startIndex, func(key string, value interface{}) bool {
items = append(items, Item{Key: key, Value: value})
return false
})
Expand Down
8 changes: 4 additions & 4 deletions examples/gno.land/p/n2p5/mgroup/mgroup_test.gno
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,13 @@ func TestManagedGroup(t *testing.T) {
if len(owners) != 3 {
t.Errorf("expected 2, got %v", len(owners))
}
if owners[0] != u2.String() {
if owners[0] != u1.String() {
t.Errorf("expected %v, got %v", u2, owners[0])
}
if owners[1] != u3.String() {
t.Errorf("expected %v, got %v", u3, owners[1])
}
if owners[2] != u1.String() {
if owners[2] != u2.String() {
t.Errorf("expected %v, got %v", u3, owners[1])
}
})
Expand All @@ -317,13 +317,13 @@ func TestManagedGroup(t *testing.T) {
if len(members) != 3 {
t.Errorf("expected 2, got %v", len(members))
}
if members[0] != u2.String() {
if members[0] != u1.String() {
t.Errorf("expected %v, got %v", u2, members[0])
}
if members[1] != u3.String() {
t.Errorf("expected %v, got %v", u3, members[1])
}
if members[2] != u1.String() {
if members[2] != u2.String() {
t.Errorf("expected %v, got %v", u3, members[1])
}
})
Expand Down

0 comments on commit 2e332dd

Please sign in to comment.