Merge pull request #67 from Workiva/DT-23494-wip

DT-23494 DT-23495 Strengthen test assertions and correct internal tree node state

benchmark/web_benchmarks.dart

@@ -3,7 +3,16 @@ import 'dart:html';
 import 'benchmarks.dart' as benchmarks;
 
 main() {
-  querySelector('#runButton')!.onClick.listen((_) {
+  final button = querySelector('#runButton')! as ButtonElement;
+  button.onClick.listen((_) async {
+    button.disabled = true;
+    final text = button.innerText;
+    button.innerText = 'Running';
+    await window.animationFrame;
+    await window.animationFrame;
+    await window.animationFrame;
     benchmarks.main();
+    button.disabled = false;
+    button.innerText = text;
   });
 }

lib/src/r_tree/leaf_node.dart

@@ -58,6 +58,6 @@ class LeafNode<E> extends Node<E> {
 
   clearChildren() {
     _items.clear();
-    _minimumBoundingRect = null;
+    _minimumBoundingRect = noMBR;
   }
 }

lib/src/r_tree/node.dart

@@ -30,17 +30,10 @@ abstract class Node<E> implements RTreeContributor {
   /// Parent node of this node, or null if this is the root node
   Node<E>? parent;
 
-  Rectangle? _minimumBoundingRect;
+  Rectangle _minimumBoundingRect = noMBR;
 
   /// Returns the rectangle this Node covers
-  Rectangle get rect {
-    if (_minimumBoundingRect != null) {
-      return _minimumBoundingRect!;
-    }
-
-    updateBoundingRect();
-    return _minimumBoundingRect ?? noMBR;
-  }
+  Rectangle get rect => _minimumBoundingRect;
 
   Node(this.branchFactor);
 
@@ -80,7 +73,7 @@ abstract class Node<E> implements RTreeContributor {
   /// Calculates the cost (increase to _minimumBoundingRect's area)
   /// of adding a new @item to this Node
   num expansionCost(RTreeContributor item) {
-    if (_minimumBoundingRect == null) {
+    if (_minimumBoundingRect == noMBR) {
       return _area(item.rect);
     }
 
@@ -94,23 +87,23 @@ abstract class Node<E> implements RTreeContributor {
 
   /// Adds the rectangle containing [item] to this node's covered rectangle
   include(RTreeContributor item) {
-    _minimumBoundingRect = _minimumBoundingRect == null ? item.rect : rect.boundingBox(item.rect);
+    _minimumBoundingRect = _minimumBoundingRect == noMBR ? item.rect : rect.boundingBox(item.rect);
   }
 
   /// Recalculated the bounding rectangle of this node
   Rectangle updateBoundingRect() {
     if (children.isEmpty) {
-      _minimumBoundingRect = null;
-      return noMBR;
+      _minimumBoundingRect = noMBR;
+      return _minimumBoundingRect;
     }
 
-    var newBoundingRect = children.first.rect;
-    for (var child in children.skip(1)) {
-      newBoundingRect = newBoundingRect.boundingBox(child.rect);
+    var updatedBoundingRect = children[0].rect;
+    for (var i = 1; i < children.length; i++) {
+      updatedBoundingRect = updatedBoundingRect.boundingBox(children[i].rect);
     }
 
-    _minimumBoundingRect = newBoundingRect;
-    return _minimumBoundingRect!;
+    _minimumBoundingRect = updatedBoundingRect;
+    return _minimumBoundingRect;
   }
 
   void extend(Rectangle b) {

lib/src/r_tree/non_leaf_node.dart

@@ -80,7 +80,7 @@ class NonLeafNode<E> extends Node<E> {
       removeChild(child);
     }
 
-    _recalculateHeight();
+    _updateHeightAndBounds();
   }
 
   addChild(Node<E> child) {
@@ -92,23 +92,20 @@ class NonLeafNode<E> extends Node<E> {
     super.removeChild(child);
     child.parent = null;
 
-    if (_childNodes.length == 0) {
-      _convertToLeafNode();
-    }
-
-    _recalculateHeight();
+    _updateHeightAndBounds();
   }
 
   clearChildren() {
     _childNodes.clear();
-    _minimumBoundingRect = null;
+    _minimumBoundingRect = noMBR;
   }
 
   Node<E> _getBestNodeForInsert(RTreeDatum<E> item) {
-    Node<E> bestNode = _childNodes.first;
+    Node<E> bestNode = _childNodes[0];
     num bestCost = bestNode.expansionCost(item);
 
-    for (var child in _childNodes.skip(1)) {
+    for (var i = 1; i < _childNodes.length; i++) {
+      final child = _childNodes[i];
       final tentativeCost = child.expansionCost(item);
       if (tentativeCost < bestCost) {
         bestCost = tentativeCost;
@@ -119,21 +116,13 @@ class NonLeafNode<E> extends Node<E> {
     return bestNode;
   }
 
-  _convertToLeafNode() {
-    var nonLeafParent = parent as NonLeafNode<E>?;
-    if (nonLeafParent == null) return;
-
-    var newLeafNode = LeafNode<E>(this.branchFactor);
-    newLeafNode.include(this);
-    nonLeafParent.removeChild(this);
-    nonLeafParent.addChild(newLeafNode);
-  }
-
-  _recalculateHeight() {
-    final maxChildHeight = _childNodes.fold(0, (int greatestHeight, childNode) {
-      return max(greatestHeight, childNode.height);
-    });
+  _updateHeightAndBounds() {
+    var maxChildHeight = 0;
+    for (final childNode in _childNodes) {
+      maxChildHeight = max(maxChildHeight, childNode.height);
+    }
+    this.height = 1 + maxChildHeight;
 
-    height = 1 + maxChildHeight;
+    updateBoundingRect();
   }
 }

lib/src/r_tree/r_tree.dart

@@ -108,6 +108,7 @@ class RTree<E> {
 
     node.children.add(inode);
     node.updateBoundingRect();
+    inode.parent = node;
 
     // split on node overflow; propagate upwards if necessary
     while (level >= 0) {
@@ -164,14 +165,17 @@ class RTree<E> {
     return node;
   }
 
-  Node<E> _build(List<RTreeDatum<E>> items, int left, int right, int height) {
+  Node<E> _build(List<RTreeDatum<E>> items, int left, int right, int height, [NonLeafNode<E>? parent]) {
     final N = right - left + 1;
     var M = _branchFactor;
-    Node<E> node;
+    NonLeafNode<E> node;
 
     if (N <= M) {
       // reached leaf level; return leaf
-      return LeafNode(_branchFactor, initialItems: items.sublist(left, right + 1));
+      return LeafNode(
+        _branchFactor,
+        initialItems: items.sublist(left, right + 1),
+      )..parent = parent;
     }
 
     if (height == 0) {
@@ -182,8 +186,9 @@ class RTree<E> {
       M = (N / pow(M, height - 1)).ceil();
     }
 
-    node = NonLeafNode(_branchFactor);
-    node.height = height;
+    node = NonLeafNode(_branchFactor)
+      ..height = height
+      ..parent = parent;
 
     // split the items into M mostly square tiles
 
@@ -201,7 +206,7 @@ class RTree<E> {
         final right3 = min(j + N2 - 1, right2);
 
         // pack each entry recursively
-        node.children.add(_build(items, j, right3, height - 1));
+        node.children.add(_build(items, j, right3, height - 1, node));
       }
     }
     node.updateBoundingRect();
@@ -334,6 +339,8 @@ class RTree<E> {
     NonLeafNode<E> newRoot = NonLeafNode<E>(_branchFactor, initialChildNodes: [node1, node2]);
     newRoot.height = _root.height + 1;
     _root = newRoot;
+    node1.parent = _root;
+    node2.parent = _root;
   }
 }
 

test/r_tree/non_leaf_node_test.dart

@@ -51,31 +51,6 @@ main() {
         expect(node.rect, equals(noMBR));
         expect(node.size, equals(0));
       });
-
-      test('converting an empty NonLeafNode to a LeafNode', () {
-        NonLeafNode parentNode = NonLeafNode(3);
-        NonLeafNode node = NonLeafNode(3);
-        node.parent = parentNode;
-        parentNode.addChild(node);
-        LeafNode leaf = LeafNode(3);
-        node.addChild(leaf);
-
-        var datum1 = RTreeDatum(Rectangle(0, 0, 1, 1), '');
-        var datum2 = RTreeDatum(Rectangle(0, 0, 1, 2), '');
-        node.insert(datum1);
-        node.insert(datum2);
-
-        parentNode.children.forEach((node) {
-          expect(node, isA<NonLeafNode>());
-        });
-
-        node.remove(datum1);
-        node.remove(datum2);
-
-        parentNode.children.forEach((node) {
-          expect(node, isA<LeafNode>());
-        });
-      });
     });
   });
 }