From 3b8d640cfbd18c7afbb83b8b449fa085ab67c1e2 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 9 Nov 2012 18:36:18 -0800 Subject: [PATCH] Preserve Entity immutability Reintroduced an Entity class. Entity mutations will be expressed as methods that return a new Entity. Extract a move operation from the drag behavior. Instead of updating an entity in place, the drag event produces a new entity and graph and replaces the current history version, which was created by doing a noop on dragstart. pdata is no longer used. It was previously removed from Graph, and I think it makes more sense to have a specialized Entity class as well. --- index.html | 2 +- js/iD/Connection.js | 2 +- js/iD/actions/actions.js | 10 ++++---- js/iD/actions/operations.js | 26 +++++++++++++++++---- js/iD/graph/Entity.js | 15 ++++++++++++ js/iD/graph/Graph.js | 11 ++++----- js/iD/graph/History.js | 5 ++++ js/iD/graph/pdata.js | 46 ------------------------------------- js/iD/renderer/Map.js | 32 +++++++++----------------- 9 files changed, 64 insertions(+), 85 deletions(-) create mode 100644 js/iD/graph/Entity.js delete mode 100644 js/iD/graph/pdata.js diff --git a/index.html b/index.html index d1f168fd7..e9a7bfceb 100755 --- a/index.html +++ b/index.html @@ -53,7 +53,7 @@ - + diff --git a/js/iD/Connection.js b/js/iD/Connection.js index 089ce3cc0..ffc35e786 100755 --- a/js/iD/Connection.js +++ b/js/iD/Connection.js @@ -78,7 +78,7 @@ iD.Connection = function() { if (o.lon) o.lon = parseFloat(o.lon); o._id = o.id; o.id = o.type[0] + o.id; - return o; + return iD.Entity(o); } function parse(callback) { diff --git a/js/iD/actions/actions.js b/js/iD/actions/actions.js index adba9e3ca..858ef3add 100644 --- a/js/iD/actions/actions.js +++ b/js/iD/actions/actions.js @@ -7,14 +7,14 @@ iD.actions = {}; // into operations. iD.actions._node = function(ll) { - return { + return iD.Entity({ type: 'node', lat: ll[1], lon: ll[0], id: iD.Util.id('node'), tags: {} - }; -}, + }); +}; iD.actions.AddPlace = { enter: function() { @@ -58,7 +58,7 @@ iD.actions.AddPlace = { iD.actions.AddRoad = { way: function(ll) { - return { + return iD.Entity({ type: 'way', nodes: [], tags: { @@ -66,7 +66,7 @@ iD.actions.AddRoad = { }, modified: true, id: iD.Util.id('way') - }; + }); }, enter: function() { d3.selectAll('button').classed('active', false); diff --git a/js/iD/actions/operations.js b/js/iD/actions/operations.js index f3c47e9d5..2468a5d73 100644 --- a/js/iD/actions/operations.js +++ b/js/iD/actions/operations.js @@ -1,5 +1,11 @@ iD.operations = {}; +iD.operations.noop = function() { + return function(graph) { + return graph; + }; +}; + iD.operations.addNode = function(node) { return function(graph) { return graph.replace(node, 'added a place'); @@ -20,16 +26,26 @@ iD.operations.remove = function(node) { iD.operations.changeWayNodes = function(way, node) { return function(graph) { - way.nodes = way.nodes.slice(); - way = pdata.object(way).get(); - return graph.replace(way).replace(node, 'added to a road'); + return graph.replace(way.update({ + nodes: way.nodes.slice() + })).replace(node, 'added to a road'); }; }; iD.operations.changeTags = function(node, tags) { return function(graph) { - var node = pdata.object(node).set({ tags: tags }).get(); - return graph.replace(node, 'changed tags'); + return graph.replace(node.update({ + tags: tags + }), 'changed tags'); + }; +}; + +iD.operations.move = function(entity, to) { + return function(graph) { + return graph.replace(entity.update({ + lon: to.lon || to[0], + lat: to.lat || to[1] + }), 'moved an element'); }; }; diff --git a/js/iD/graph/Entity.js b/js/iD/graph/Entity.js new file mode 100644 index 000000000..acbdb1dd6 --- /dev/null +++ b/js/iD/graph/Entity.js @@ -0,0 +1,15 @@ +iD.Entity = function(a, b) { + if (!(this instanceof iD.Entity)) return new iD.Entity(a, b); + + _.extend(this, a, b); + + if (b) { + this.modified = true; + } +}; + +iD.Entity.prototype = { + update: function(attrs) { + return iD.Entity(this, attrs); + } +}; diff --git a/js/iD/graph/Graph.js b/js/iD/graph/Graph.js index fe34e9fa6..96f886336 100644 --- a/js/iD/graph/Graph.js +++ b/js/iD/graph/Graph.js @@ -56,12 +56,11 @@ iD.Graph.prototype = { // Resolve the id references in a way, replacing them with actual objects. fetch: function(id) { - var o = this.entities[id]; - var f = _.clone(o); - if (!f.nodes || !f.nodes.length) return f; - f.nodes = f.nodes.map(function(c) { - return this.fetch(c); + var entity = iD.Entity(this.entities[id]); + if (!entity.nodes || !entity.nodes.length) return entity; + entity.nodes = entity.nodes.map(function(id) { + return this.fetch(id); }.bind(this)); - return f; + return entity; } }; diff --git a/js/iD/graph/History.js b/js/iD/graph/History.js index c4f04f8a1..cac4772f7 100644 --- a/js/iD/graph/History.js +++ b/js/iD/graph/History.js @@ -21,6 +21,11 @@ iD.History.prototype = { this.index++; }, + replace: function(operation) { + // assert(this.index == this.stack.length - 1) + this.stack[this.index] = operation(this.graph()); + }, + undo: function() { while (this.index > 0) { this.index--; diff --git a/js/iD/graph/pdata.js b/js/iD/graph/pdata.js deleted file mode 100644 index 101d2b455..000000000 --- a/js/iD/graph/pdata.js +++ /dev/null @@ -1,46 +0,0 @@ -var pdata = {}; - -pdata.object = function(input) { - - var v = clone(input), - proxy = {}; - - function clone(x) { - var v = {}; - for (var k in x) v[k] = x[k]; - return v; - } - - // Remove a key from the object. This is like `delete`, - // but does not delete the key in this closure's object - proxy.remove = function(key) { - var n = {}, k, i; - if (typeof key === 'object') { - var keys = {}; - for (i = 0; i < key.length; i++) keys[key[i]] = true; - for (k in v) if (!keys[k]) n[k] = v[k]; - } else { - for (k in v) if (k !== key) n[k] = v[k]; - } - return pdata.object(n); - }; - - // Set a value or values in the object. Overwrites - // existing values. - proxy.set = function(vals) { - var n = clone(v); - for (var j in vals) { - n[j] = vals[j]; - } - return pdata.object(n); - }; - - // Get the contained object. - proxy.get = function() { - return clone(v); - }; - - return proxy; -}; - -if (typeof module !== 'undefined') module.exports = pdata; diff --git a/js/iD/renderer/Map.js b/js/iD/renderer/Map.js index ed1dc59c5..528570216 100755 --- a/js/iD/renderer/Map.js +++ b/js/iD/renderer/Map.js @@ -37,29 +37,19 @@ iD.Map = function(elem) { .on('zoom', zoomPan), // this is used with handles dragbehavior = d3.behavior.drag() - .origin(function(d) { - var entity = (typeof d === 'string') ? history.entity(d) : d; - history.do(function(graph) { - var node = pdata.object(entity).set({ modified: true }).get(); - return graph.replace(node); - }); + .origin(function(entity) { var p = projection(ll2a(entity)); return { x: p[0], y: p[1] }; }) - .on('drag', function(d) { - d3.select(this).attr('transform', function() { - return 'translate(' + d3.event.x + ',' + d3.event.y + ')'; - }); - var ll = projection.invert([d3.event.x, d3.event.y]); - history.entity(d).lon = ll[0]; - history.entity(d).lat = ll[1]; + .on('dragstart', function() { + history.do(iD.operations.noop()); + }) + .on('drag', function(entity) { + var to = projection.invert([d3.event.x, d3.event.y]); + history.replace(iD.operations.move(entity, to)); drawVector(); }) - .on('dragend', function(d) { - var entity = (typeof d === 'string') ? history.entity(d) : d; - history.do(function(graph) { - return graph.replace(entity, 'moved an element'); - }); + .on('dragend', function() { map.update(); }), // geo @@ -181,15 +171,15 @@ iD.Map = function(elem) { }); var handles = hit_g.selectAll('circle.handle') - .data(selection.length ? (active_entity.length ? active_entity[0].nodes : []) : []); + .data(active_entity.length ? graph.fetch(active_entity[0].id).nodes : []); handles.exit().remove(); handles.enter().append('circle') .attr('class', 'handle') .attr('r', 5) .call(dragbehavior); - handles.attr('transform', function(d) { - return 'translate(' + projection(ll2a(graph.entity(d))) + ')'; + handles.attr('transform', function(entity) { + return 'translate(' + projection(ll2a(entity)) + ')'; }); }