From f6d144c151f071fba21707337c21b4d31b8cc9e6 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 1 Jan 2015 22:49:44 -0500 Subject: [PATCH] Improvements, and simplify code * don't worry about deep copying, because immutability * don't need `attrs` parameter which is usually empty * don't worry about resetting `v` entity version --- js/id/actions/copy_entity.js | 2 +- js/id/core/entity.js | 15 ++---------- js/id/core/relation.js | 45 ++++++++++++++++++------------------ js/id/core/way.js | 43 +++++++++++++++++----------------- test/spec/core/entity.js | 6 ++--- test/spec/core/relation.js | 3 +-- test/spec/core/way.js | 3 +-- 7 files changed, 52 insertions(+), 65 deletions(-) diff --git a/js/id/actions/copy_entity.js b/js/id/actions/copy_entity.js index 3db8678b4..63b9eeffe 100644 --- a/js/id/actions/copy_entity.js +++ b/js/id/actions/copy_entity.js @@ -1,6 +1,6 @@ iD.actions.CopyEntity = function(entity, deep) { return function(graph) { - var newEntities = entity.copy({}, deep, graph); + var newEntities = entity.copy(deep, graph); for (var i = 0, imax = newEntities.length; i !== imax; i++) { graph = graph.replace(newEntities[i]); diff --git a/js/id/core/entity.js b/js/id/core/entity.js index 5b2412623..f4f52274c 100644 --- a/js/id/core/entity.js +++ b/js/id/core/entity.js @@ -69,21 +69,10 @@ iD.Entity.prototype = { return this; }, - copy: function(attrs) { - var clone = {}, - omit = {}; - - _.each(['tags', 'loc', 'nodes', 'members'], function(prop) { - if (this.hasOwnProperty(prop)) clone[prop] = _.cloneDeep(this[prop]); - }, this); - - _.each(['id', 'user', 'v', 'version'], function(prop) { - omit[prop] = undefined; - }); - + copy: function() { // Returns an array so that we can support deep copying ways and relations. // The first array element will contain this.copy, followed by any descendants. - return [ iD.Entity(this, _.extend(clone, (attrs || {}), omit)) ]; + return [iD.Entity(this, {id: undefined, user: undefined, version: undefined})]; }, osmId: function() { diff --git a/js/id/core/relation.js b/js/id/core/relation.js index ebafd89c1..174904cad 100644 --- a/js/id/core/relation.js +++ b/js/id/core/relation.js @@ -20,30 +20,31 @@ _.extend(iD.Relation.prototype, { type: 'relation', members: [], - copy: function(attrs, deep, resolver) { - var fn = iD.Entity.prototype.copy; + copy: function(deep, resolver) { + var copy = iD.Entity.prototype.copy.call(this); - if (deep && resolver && this.isComplete(resolver)) { - var members = [], - descendants = [], - replacements = {}, - i, oldmember, oldid, newid, child; - - for (i = 0; i < this.members.length; i++) { - oldmember = this.members[i]; - oldid = oldmember.id; - newid = replacements[oldid]; - if (!newid) { - child = resolver.entity(oldid).copy({}, true, resolver); - newid = replacements[oldid] = child[0].id; - descendants = child.concat(descendants); - } - members.push({id: newid, type: oldmember.type, role: oldmember.role}); - } - return fn.call(this, _.extend(attrs, {members: members})).concat(descendants); - } else { - return fn.call(this, attrs); + if (!deep || !resolver || !this.isComplete(resolver)) { + return copy; } + + var members = [], + replacements = {}, + i, oldmember, oldid, newid, children; + + for (i = 0; i < this.members.length; i++) { + oldmember = this.members[i]; + oldid = oldmember.id; + newid = replacements[oldid]; + if (!newid) { + children = resolver.entity(oldid).copy(true, resolver); + newid = replacements[oldid] = children[0].id; + copy = copy.concat(children); + } + members.push({id: newid, type: oldmember.type, role: oldmember.role}); + } + + copy[0] = copy[0].update({members: members}); + return copy; }, extent: function(resolver, memo) { diff --git a/js/id/core/way.js b/js/id/core/way.js index f31ebfc25..7fad36eba 100644 --- a/js/id/core/way.js +++ b/js/id/core/way.js @@ -12,29 +12,30 @@ _.extend(iD.Way.prototype, { type: 'way', nodes: [], - copy: function(attrs, deep, resolver) { - var fn = iD.Entity.prototype.copy; + copy: function(deep, resolver) { + var copy = iD.Entity.prototype.copy.call(this); - if (deep && resolver) { - var nodes = [], - descendants = [], - replacements = {}, - i, oldid, newid, child; - - for (i = 0; i < this.nodes.length; i++) { - oldid = this.nodes[i]; - newid = replacements[oldid]; - if (!newid) { - child = resolver.entity(oldid).copy(); - newid = replacements[oldid] = child[0].id; - descendants = child.concat(descendants); - } - nodes.push(newid); - } - return fn.call(this, _.extend(attrs, {nodes: nodes})).concat(descendants); - } else { - return fn.call(this, attrs); + if (!deep || !resolver) { + return copy; } + + var nodes = [], + replacements = {}, + i, oldid, newid, child; + + for (i = 0; i < this.nodes.length; i++) { + oldid = this.nodes[i]; + newid = replacements[oldid]; + if (!newid) { + child = resolver.entity(oldid).copy(); + newid = replacements[oldid] = child[0].id; + copy = copy.concat(child); + } + nodes.push(newid); + } + + copy[0] = copy[0].update({nodes: nodes}); + return copy; }, extent: function(resolver) { diff --git a/test/spec/core/entity.js b/test/spec/core/entity.js index 2c0efb487..5a6e0aa79 100644 --- a/test/spec/core/entity.js +++ b/test/spec/core/entity.js @@ -45,19 +45,17 @@ describe('iD.Entity', function () { expect(a).not.to.equal(result[0]); }); - it("resets 'id', 'user', 'v', and 'version' properties", function () { - var a = iD.Entity({id: 'n1234', version: 10, v: 4, user: 'bot-mode'}), + it("resets 'id', 'user', and 'version' properties", function () { + var a = iD.Entity({id: 'n1234', version: 10, user: 'bot-mode'}), b = a.copy()[0]; expect(b.isNew()).to.be.ok; expect(b.version).to.be.undefined; - expect(b.v).to.be.undefined; expect(b.user).to.be.undefined; }); it("copies tags", function () { var a = iD.Entity({id: 'n1234', version: 10, user: 'test', tags: {foo: 'foo'}}), b = a.copy()[0]; - expect(b.tags).not.to.equal(a.tags); expect(b.tags).to.deep.equal(a.tags); }); }); diff --git a/test/spec/core/relation.js b/test/spec/core/relation.js index 12593100b..cd13d2dd4 100644 --- a/test/spec/core/relation.js +++ b/test/spec/core/relation.js @@ -48,7 +48,6 @@ describe('iD.Relation', function () { r2 = result[0]; expect(result).to.have.length(1); - expect(r1.members).not.to.equal(r2.members); expect(r1.members).to.deep.equal(r2.members); }); @@ -59,7 +58,7 @@ describe('iD.Relation', function () { w1 = iD.Way({id: 'w1', nodes: ['a','b','c','a']}), r1 = iD.Relation({id: 'r1', members: [{id: 'w1', role: 'outer'}]}), graph = iD.Graph([a, b, c, w1, r1]), - result = r1.copy({}, true, graph), + result = r1.copy(true, graph), r2 = result[0]; expect(result).to.have.length(5); diff --git a/test/spec/core/way.js b/test/spec/core/way.js index d3bdc57a1..274835fb1 100644 --- a/test/spec/core/way.js +++ b/test/spec/core/way.js @@ -47,7 +47,6 @@ describe('iD.Way', function() { w2 = result[0]; expect(result).to.have.length(1); - expect(w1.nodes).not.to.equal(w2.nodes); expect(w1.nodes).to.deep.equal(w2.nodes); }); @@ -57,7 +56,7 @@ describe('iD.Way', function() { c = iD.Node({id: 'c'}), w1 = iD.Entity({id: 'w1', nodes: ['a','b','c','a']}), graph = iD.Graph([a, b, c, w1]), - result = w1.copy({}, true, graph), + result = w1.copy(true, graph), w2 = result[0]; expect(result).to.have.length(4);