Skip to content

Commit 08a5860

Browse files
committed
Handle nil child slots consistently
Treat nil child slots as empty in children? and siblings to avoid phantom entries in binary trees. Add regression coverage and note the behavior change in API-CHANGES.
1 parent 5ba13a2 commit 08a5860

3 files changed

Lines changed: 39 additions & 6 deletions

File tree

API-CHANGES.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ _Note_: API changes are expected to reduce significantly after the `1.x`
77
release. In most cases, an alternative will be provided to ensure relatively
88
smooth transition to the new APIs.
99

10+
## Release 3.0.0 Changes
11+
12+
* Minimum Ruby version is now 3.1 (support for 2.7 and 3.0 has been dropped).
13+
14+
* [Tree::TreeNode#children?][children] and [Tree::TreeNode#siblings][siblings]
15+
now treat `nil` child slots as empty, preventing binary trees with missing
16+
children from reporting or yielding phantom siblings.
17+
1018
## Release 2.2.0 Changes
1119

1220
* [Tree::TreeNode#add][add] now raises `ArgumentError` when attempting to add
@@ -29,10 +37,6 @@ smooth transition to the new APIs.
2937
* [Tree::TreeNode#each_level][each_level] now returns a level-wise enumerator
3038
when called without a block.
3139

32-
## Release 3.0.0 Changes
33-
34-
* Minimum Ruby version is now 3.1 (support for 2.7 and 3.0 has been dropped).
35-
3640
## Release 2.1.0 Changes
3741

3842
* Minimum Ruby version has been bumped to 2.7 and above
@@ -165,6 +169,7 @@ smooth transition to the new APIs.
165169
[append]: rdoc-ref:Tree::TreeNode#<<
166170
[breadth_each]: rdoc-ref:Tree::TreeNode#breadth_each
167171
[btree_add]: rdoc-ref:Tree::BinaryTreeNode#add
172+
[children]: rdoc-ref:Tree::TreeNode#children?
168173
[depth]: rdoc-ref:Tree::Utils::TreeMetricsHandler#depth
169174
[detached_subtree_copy]: rdoc-ref:Tree::TreeNode#detached_subtree_copy
170175
[dup]: rdoc-ref:Tree::TreeNode#dup

lib/tree.rb

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ class TreeNode
127127
# Root node for the (sub)tree to which this node belongs.
128128
# A root node's root is itself.
129129
#
130+
# @note This walks the parent chain each call; if this is on a hot path,
131+
# consider caching the result at the caller.
132+
#
130133
# @return [Tree::TreeNode] Root of the (sub)tree.
131134
def root
132135
root = self
@@ -191,11 +194,13 @@ def parentage
191194
# @!attribute [r] children?
192195
# +true+ if the this node has any child node.
193196
#
197+
# @note Nil child slots (e.g., in binary trees) do not count as children.
198+
#
194199
# @return [Boolean] +true+ if child nodes exist.
195200
#
196201
# @see #leaf?
197202
def children?
198-
!@children.empty?
203+
@children.any?
199204
end
200205

201206
alias has_children? children? # @todo: Aliased for eventual replacement
@@ -879,6 +884,8 @@ def last_sibling?
879884
# If a block is provided, yields each of the sibling nodes to the block.
880885
# The root always has +nil+ siblings.
881886
#
887+
# @note Nil child slots (e.g., in binary trees) are skipped.
888+
#
882889
# @yieldparam sibling [Tree::TreeNode] Each sibling node.
883890
#
884891
# @return [Array<Tree::TreeNode>] Array of siblings of this node. Will
@@ -890,13 +897,19 @@ def last_sibling?
890897
# @see #last_sibling
891898
def siblings
892899
if block_given?
893-
parent.children.each { |sibling| yield sibling if sibling != self }
900+
parent.children.each do |sibling|
901+
next unless sibling
902+
next if sibling == self
903+
904+
yield sibling
905+
end
894906
self
895907
else
896908
return [] if root?
897909

898910
siblings = []
899911
parent.children do |my_sibling|
912+
next unless my_sibling
900913
siblings << my_sibling if my_sibling != self
901914
end
902915
siblings

test/test_binarytree.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,21 @@ def test_left_child_equals
236236
assert_nil(@root['A Child at Left'], 'Lookup by old left name should be nil')
237237
end
238238

239+
def test_nil_children_slots
240+
@root << @left_child1
241+
@root << @right_child1
242+
243+
@root.left_child = nil
244+
assert(@root.children?, 'Root should still have a right child')
245+
246+
collected = []
247+
@root.right_child.siblings { |sibling| collected << sibling }
248+
assert_equal([], collected, 'Nil sibling slots should not be yielded')
249+
250+
@root.right_child = nil
251+
assert(!@root.children?, 'Root should report no children once all slots are nil')
252+
end
253+
239254
# Test right_child= method.
240255
def test_right_child_equals
241256
@root << @left_child1

0 commit comments

Comments
 (0)