Encapsulating the SweepLine

Defensively programming around w8r/avl#15

src/subdivide-segments.js

@@ -1,5 +1,4 @@
-const Tree = require('avl')
-const compareSegments = require('./compare-segments')
+const SweepLine = require('./sweep-line')
 
 const possibleIntersection = (se1, se2) => {
   const inters = se1.segment.getIntersections(se2.segment)
@@ -19,20 +18,17 @@ const possibleIntersection = (se1, se2) => {
 }
 
 module.exports = eventQueue => {
-  const sweepLine = new Tree(compareSegments)
+  const sweepLine = new SweepLine()
   const sortedEvents = []
 
   while (!eventQueue.isEmpty) {
     const event = eventQueue.pop()
     sortedEvents.push(event)
 
     if (event.isLeft) {
-      const eventNode = sweepLine.insert(event)
-      const prevNode = sweepLine.prev(eventNode)
-      const nextNode = sweepLine.next(eventNode)
-
-      const prevEvent = prevNode ? prevNode.key : null
-      const nextEvent = nextNode ? nextNode.key : null
+      const node = sweepLine.insert(event)
+      const prevEvent = sweepLine.prevKey(node)
+      const nextEvent = sweepLine.nextKey(node)
 
       event.registerPrevEvent(prevEvent)
 
@@ -42,9 +38,8 @@ module.exports = eventQueue => {
 
     if (event.isRight) {
       const leftEvent = event.otherSE
-      const leftNode = sweepLine.find(leftEvent)
-      const nextNode = sweepLine.next(leftNode)
-      const nextEvent = nextNode ? nextNode.key : null
+      const node = sweepLine.find(leftEvent)
+      const nextEvent = sweepLine.nextKey(node)
 
       if (nextEvent && leftEvent.segment.isCoincidentWith(nextEvent.segment)) {
         leftEvent.registerCoincidentEvent(nextEvent, true)

src/sweep-line.js

@@ -0,0 +1,62 @@
+const Tree = require('avl')
+const compareSegments = require('./compare-segments')
+
+/**
+ * NOTE:  It appears if you pull out a node from the AVL tree,
+ *        then do a remove() on the tree, the nodes can get
+ *        messed up: https://github.com/w8r/avl/issues/15
+ *
+ *        As such, the methods here which accept nodes back from
+ *        the client will throw an exception if remove() has been
+ *        called since that node was first given to the client.
+ */
+
+class SweepLine {
+  constructor (comparator = compareSegments) {
+    this.tree = new Tree(comparator)
+    this.removeCounter = 1
+  }
+
+  /* Returns the new node associated with the key */
+  insert (key) {
+    const node = this.tree.insert(key)
+    return this._annotateNode(node)
+  }
+
+  /* Returns the node associated with the key */
+  find (key, returnNeighbors = false) {
+    const node = this.tree.find(key)
+    return this._annotateNode(node)
+  }
+
+  prevKey (node) {
+    this._checkNode(node)
+    const prevNode = this.tree.prev(node)
+    return prevNode ? prevNode.key : null
+  }
+
+  nextKey (node) {
+    this._checkNode(node)
+    const nextNode = this.tree.next(node)
+    return nextNode ? nextNode.key : null
+  }
+
+  remove (key) {
+    this.removeCounter++
+    this.tree.remove(key)
+  }
+
+  _checkNode (node) {
+    /* defensively working around https://github.com/w8r/avl/issues/15 */
+    if (node.removeCounter !== this.removeCounter) {
+      throw new Error('Tried to use stale node')
+    }
+  }
+
+  _annotateNode (node) {
+    if (node !== null) node.removeCounter = this.removeCounter
+    return node
+  }
+}
+
+module.exports = SweepLine

test/sweep-line.test.js

@@ -1,43 +1,91 @@
 /* eslint-env jest */
 
-const Tree = require('avl')
-const compareSegments = require('../src/compare-segments')
-const Segment = require('../src/segment')
+const SweepLine = require('../src/sweep-line')
+
+const comparator = (a, b) => {
+  if (a === b) return 0
+  return a < b ? -1 : 1
+}
 
 describe('sweep line', () => {
-  const s = [[[16, 282], [298, 359], [153, 203.5], [16, 282]]]
-  const c = [[[56, 181], [153, 294.5], [241.5, 229.5], [108.5, 120], [56, 181]]]
-
-  test('general', () => {
-    const EF = new Segment(s[0][0], s[0][2], true).leftSE
-    const EG = new Segment(s[0][0], s[0][1], true).leftSE
-
-    const tree = new Tree(compareSegments)
-    tree.insert(EF)
-    tree.insert(EG)
-
-    expect(tree.find(EF).key).toBe(EF)
-    expect(tree.minNode().key).toBe(EF)
-    expect(tree.maxNode().key).toBe(EG)
-
-    let it = tree.find(EF)
-    expect(tree.next(it).key).toBe(EG)
-    it = tree.find(EG)
-    expect(tree.prev(it).key).toBe(EF)
-
-    const DA = new Segment(c[0][0], c[0][2], true).leftSE
-    const DC = new Segment(c[0][0], c[0][1], true).leftSE
-
-    tree.insert(DA)
-    tree.insert(DC)
-
-    let node = tree.minNode()
-    expect(node.key).toBe(DA)
-    node = tree.next(node)
-    expect(node.key).toBe(DC)
-    node = tree.next(node)
-    expect(node.key).toBe(EF)
-    node = tree.next(node)
-    expect(node.key).toBe(EG)
+  test('fill it up then empty it out', () => {
+    const sl = new SweepLine(comparator)
+    const k1 = 4
+    const k2 = 9
+    const k3 = 13
+    const k4 = 44
+
+    let n1 = sl.insert(k1)
+    let n2 = sl.insert(k2)
+    let n4 = sl.insert(k4)
+    let n3 = sl.insert(k3)
+
+    expect(sl.find(k1)).toBe(n1)
+    expect(sl.find(k2)).toBe(n2)
+    expect(sl.find(k3)).toBe(n3)
+    expect(sl.find(k4)).toBe(n4)
+
+    expect(sl.prevKey(n1)).toBeNull()
+    expect(sl.nextKey(n1)).toBe(k2)
+
+    expect(sl.prevKey(n2)).toBe(k1)
+    expect(sl.nextKey(n2)).toBe(k3)
+
+    expect(sl.prevKey(n3)).toBe(k2)
+    expect(sl.nextKey(n3)).toBe(k4)
+
+    expect(sl.prevKey(n4)).toBe(k3)
+    expect(sl.nextKey(n4)).toBeNull()
+
+    sl.remove(k2)
+    expect(sl.find(k2)).toBeNull()
+
+    expect(() => sl.nextKey(n1)).toThrow()
+    expect(() => sl.nextKey(n2)).toThrow()
+    expect(() => sl.nextKey(n3)).toThrow()
+    expect(() => sl.nextKey(n4)).toThrow()
+
+    n1 = sl.find(k1)
+    n3 = sl.find(k3)
+    n4 = sl.find(k4)
+
+    expect(sl.prevKey(n1)).toBeNull()
+    expect(sl.nextKey(n1)).toBe(k3)
+
+    expect(sl.prevKey(n3)).toBe(k1)
+    expect(sl.nextKey(n3)).toBe(k4)
+
+    expect(sl.prevKey(n4)).toBe(k3)
+    expect(sl.nextKey(n4)).toBeNull()
+
+    sl.remove(k4)
+    expect(sl.find(k4)).toBeNull()
+
+    expect(() => sl.prevKey(n1)).toThrow()
+    expect(() => sl.prevKey(n3)).toThrow()
+    expect(() => sl.prevKey(n4)).toThrow()
+
+    n1 = sl.find(k1)
+    n3 = sl.find(k3)
+
+    expect(sl.prevKey(n1)).toBeNull()
+    expect(sl.nextKey(n1)).toBe(k3)
+
+    expect(sl.prevKey(n3)).toBe(k1)
+    expect(sl.nextKey(n3)).toBeNull()
+
+    sl.remove(k1)
+    expect(sl.find(k1)).toBeNull()
+
+    expect(() => sl.nextKey(n1)).toThrow()
+    expect(() => sl.nextKey(n4)).toThrow()
+
+    n3 = sl.find(k3)
+
+    expect(sl.prevKey(n3)).toBeNull()
+    expect(sl.nextKey(n3)).toBeNull()
+
+    sl.remove(k3)
+    expect(sl.find(k3)).toBeNull()
   })
 })