From fa70d79622ce9d1c24b542963b78bc6618b6ee14 Mon Sep 17 00:00:00 2001 From: slhh Date: Fri, 16 Dec 2016 15:38:59 +0100 Subject: [PATCH 1/2] Additional tests added for addNode, updateNode, and replaceNode. --- test/spec/osm/way.js | 116 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index a49ea4082..afc4fa90f 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -410,11 +410,21 @@ describe('iD.osmWay', function() { }); describe('#addNode', function () { - it('adds a node to the end of a way', function () { + it('adds a node to the end of a way when index is undefined', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c').nodes).to.eql(['a', 'b', 'c']); + }); + + it('adds a node to an empty way', function () { var w = iD.Way(); expect(w.addNode('a').nodes).to.eql(['a']); }); + it('adds a node to the end of a way at a index greater than length', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c',3).nodes).to.eql(['a', 'b', 'c']); + }); + it('adds a node to a way at index 0', function () { var w = iD.Way({nodes: ['a', 'b']}); expect(w.addNode('c', 0).nodes).to.eql(['c', 'a', 'b']); @@ -429,6 +439,36 @@ describe('iD.osmWay', function() { var w = iD.Way({nodes: ['a', 'b']}); expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']); }); + + it('prevents duplicate consecutive nodes when adding in front of', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('b', 1).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes when adding behind', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('a', 1).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes at index 0', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('a', 0).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes at a negative index', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('a', -1).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes at a index equal to length', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('b', 2).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes when index is undefined', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('b').nodes).to.eql(['a', 'b']); + }); }); describe('#updateNode', function () { @@ -436,8 +476,74 @@ describe('iD.osmWay', function() { var w = iD.Way({nodes: ['a', 'b', 'c']}); expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']); }); + + it('prevents duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.updateNode('b',2).nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.updateNode('b',2).nodes).to.eql(['a', 'b','e']); + }); + + it('preserves duplicate non-consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'b','e']); + }); + + it('replaces a single one of duplicate nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.updateNode('d',1).nodes).to.eql(['a', 'd', 'c', 'b','e']); + w = iD.Way({nodes: ['a', 'b', 'b', 'c','e']}); + expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'c','e']); + }); + + it('removes existing duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.updateNode('c',5).nodes).to.eql(['a', 'b', 'd', 'b','c']); + w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.updateNode('c',3).nodes).to.eql(['a', 'b', 'c', 'b', 'e']); + }); }); + describe('#replaceNode', function () { + it('replaces the node', function () { + var w = iD.Way({nodes: ['a']}); + expect(w.replaceNode('a','b').nodes).to.eql(['b']); + w = iD.Way({nodes: ['a', 'b', 'c']}); + expect(w.replaceNode('b', 'd').nodes).to.eql(['a', 'd', 'c']); + }); + + it('prevents duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b','e']); + }); + + it('preserves duplicate non-consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd', 'b','e']); + }); + + it('replaces duplicate non-consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd','e']); + }); + + it('removes existing duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.replaceNode('e','c').nodes).to.eql(['a', 'b', 'd', 'b','c']); + w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.replaceNode('d','c').nodes).to.eql(['a', 'b', 'c', 'b', 'e']); + w = iD.Way({nodes: ['a', 'b', 'b', 'c', 'b', 'e']}); + expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd', 'e']); + }); + }); + + describe('#removeNode', function () { it('removes the node', function () { var w = iD.Way({nodes: ['a']}); @@ -457,6 +563,14 @@ describe('iD.osmWay', function() { it('prevents duplicate consecutive nodes when preserving circularity', function () { var w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'b', 'a']}); expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']); + w = iD.Way({nodes: ['a', 'b', 'a']}); + expect(w.removeNode('b').nodes).to.eql(['a']); + }); + it('removes existing duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.removeNode('e').nodes).to.eql(['a', 'b', 'd', 'b']); + w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.removeNode('b').nodes).to.eql(['a', 'd', 'e']); }); }); From 7817d6dff113860a6d2425e725b600e58881dd89 Mon Sep 17 00:00:00 2001 From: slhh Date: Fri, 16 Dec 2016 15:43:29 +0100 Subject: [PATCH 2/2] Prevent duplicate consecutive nodes in addNode, updateNode, and replaceNode, and add some comments. --- modules/osm/way.js | 82 +++++++++++++++++++++++++++++++++------------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index 7f328894c..e6c7a4c67 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -187,34 +187,70 @@ _.extend(osmWay.prototype, { }); }, - + // Adds a node (id) in front of the node which is currently at position index. + // If index is negative, it will be counted from the end of the way. + // If index is 0 or < -length, the node (id) will be added at the start of the way. + // If index is undefined or >= length, the node (id) will be added at the end of the way. + // Generating consecutive duplicates is silently prevented + addNode: function(id, index) { - var nodes = this.nodes.slice(); - nodes.splice(index === undefined ? nodes.length : index, 0, id); - return this.update({nodes: nodes}); - }, - - - updateNode: function(id, index) { - var nodes = this.nodes.slice(); - nodes.splice(index, 1, id); - return this.update({nodes: nodes}); - }, - - - replaceNode: function(needle, replacement) { - if (this.nodes.indexOf(needle) < 0) - return this; - - var nodes = this.nodes.slice(); - for (var i = 0; i < nodes.length; i++) { - if (nodes[i] === needle) { - nodes[i] = replacement; - } + var nodes = this.nodes.slice(), + spliceIndex = index === undefined ? nodes.length : index; + if (spliceIndex > nodes.length) spliceIndex = nodes.length; + if (spliceIndex < 0) spliceIndex = nodes.length + index; + if (spliceIndex < 0) spliceIndex = 0; + if (nodes[spliceIndex] !== id&& nodes[spliceIndex-1] !== id) { + nodes.splice(spliceIndex, 0, id); } return this.update({nodes: nodes}); }, + // Replaces the node which is currently at position index with the given node (id). + // If index is negative, it will be counted from the end of the way. + + // Consecutive duplicates are eliminated including existing ones. + + updateNode: function(id, index) { + var nodes = []; + + if (index < 0) index = this.nodes.length + index; + + for (var i = 0; i < this.nodes.length; i++) { + var node = this.nodes[i]; + if (i === index) { + if (nodes[nodes.length - 1] !== id) + nodes.push(id); + } else { + if (nodes[nodes.length - 1] !== node) + nodes.push(node); + } + } + + return this.update({nodes: nodes}); + }, + + // Replaces each occurrence of node id needle with replacement. + // Consecutive duplicates are eliminated including existing ones. + + replaceNode: function(needle, replacement) { + var nodes = []; + + for (var i = 0; i < this.nodes.length; i++) { + var node = this.nodes[i]; + if (node === needle) { + if (nodes[nodes.length - 1] !== replacement) + nodes.push(replacement); + } else { + if (nodes[nodes.length - 1] !== node) + nodes.push(node); + } + } + + return this.update({nodes: nodes}); + }, + + // Removes each occurrence of node id needle with replacement. + // Consecutive duplicates are eliminated. Circularity is preserved. removeNode: function(id) { var nodes = [];