Converting some actions to entity methods

The guidelines here are:

Entity methods:
  return a modified entity
  don't necessarily maintain whole-graph consistency

Actions:
  return a modified graph
  always maintain whole-graph consistency
  call entity methods liberally
  generally don't call other actions
This commit is contained in:
John Firebaugh
2013-01-23 10:36:02 -05:00
parent eefa22531c
commit 292c916cb1
25 changed files with 143 additions and 179 deletions

View File

@@ -71,7 +71,6 @@
<script src='js/id/actions.js'></script>
<script src='js/id/actions/add_node.js'></script>
<script src='js/id/actions/add_relation_member.js'></script>
<script src='js/id/actions/add_way.js'></script>
<script src='js/id/actions/add_way_node.js'></script>
<script src='js/id/actions/change_entity_tags.js'></script>
@@ -80,12 +79,9 @@
<script src='js/id/actions/move_node.js'></script>
<script src='js/id/actions/move_way.js'></script>
<script src='js/id/actions/noop.js'></script>
<script src='js/id/actions/remove_relation_member.js'></script>
<script src='js/id/actions/remove_way_node.js'></script>
<script src='js/id/actions/reverse_way.js'></script>
<script src='js/id/actions/split_way.js'></script>
<script src='js/id/actions/unjoin_node.js'></script>
<script src='js/id/actions/update_relation_member.js'></script>
<script src='js/id/behavior.js'></script>
<script src='js/id/behavior/drag.js'></script>

View File

@@ -1,9 +0,0 @@
iD.actions.AddRelationMember = function(relationId, member, index) {
return function(graph) {
var relation = graph.entity(relationId),
members = relation.members.slice();
members.splice((index === undefined) ? members.length : index, 0, member);
return graph.replace(relation.update({members: members}));
};
};

View File

@@ -1,10 +1,6 @@
// https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as
iD.actions.AddWayNode = function(wayId, nodeId, index) {
return function(graph) {
var way = graph.entity(wayId),
node = graph.entity(nodeId),
nodes = way.nodes.slice();
nodes.splice((index === undefined) ? nodes.length : index, 0, nodeId);
return graph.replace(way.update({nodes: nodes}));
return graph.replace(graph.entity(wayId).addNode(nodeId, index));
};
};

View File

@@ -5,12 +5,12 @@ iD.actions.DeleteNode = function(nodeId) {
graph.parentWays(node)
.forEach(function(parent) {
graph = iD.actions.RemoveWayNode(parent.id, nodeId)(graph);
graph = graph.replace(parent.removeNode(nodeId));
});
graph.parentRelations(node)
.forEach(function(parent) {
graph = iD.actions.RemoveRelationMember(parent.id, nodeId)(graph);
graph = graph.replace(parent.removeMember(nodeId));
});
return graph.remove(node);

View File

@@ -5,7 +5,7 @@ iD.actions.DeleteWay = function(wayId) {
graph.parentRelations(way)
.forEach(function(parent) {
graph = iD.actions.RemoveRelationMember(parent.id, wayId)(graph);
graph = graph.replace(parent.removeMember(wayId));
});
way.nodes.forEach(function (nodeId) {
@@ -15,7 +15,7 @@ iD.actions.DeleteWay = function(wayId) {
// can be deleted on earlier iterations of this loop.
if (!node) return;
graph = iD.actions.RemoveWayNode(wayId, nodeId)(graph);
graph = graph.replace(way.removeNode(nodeId));
if (!graph.parentWays(node).length &&
!graph.parentRelations(node).length) {

View File

@@ -2,7 +2,6 @@
// https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MoveNodeAction.as
iD.actions.MoveNode = function(nodeId, loc) {
return function(graph) {
var node = graph.entity(nodeId);
return graph.replace(node.update({loc: loc}));
return graph.replace(graph.entity(nodeId).move(loc));
};
};

View File

@@ -6,7 +6,7 @@ iD.actions.MoveWay = function(wayId, delta, projection) {
var node = graph.entity(id),
start = projection(node.loc),
end = projection.invert([start[0] + delta[0], start[1] + delta[1]]);
graph = iD.actions.MoveNode(id, end)(graph);
graph = graph.replace(node.move(end));
});
return graph;

View File

@@ -1,9 +0,0 @@
iD.actions.RemoveRelationMember = function(relationId, memberId) {
return function(graph) {
var relation = graph.entity(relationId),
members = _.reject(relation.members, function(r) {
return r.id === memberId;
});
return graph.replace(relation.update({members: members}));
};
};

View File

@@ -1,17 +0,0 @@
iD.actions.RemoveWayNode = function(wayId, nodeId) {
return function(graph) {
var way = graph.entity(wayId), nodes;
// If this is the connecting node in a closed area
if (way.nodes.length > 1 &&
_.indexOf(way.nodes, nodeId) === 0 &&
_.lastIndexOf(way.nodes, nodeId) === way.nodes.length - 1) {
// Remove the node
nodes = _.without(way.nodes, nodeId);
// And reclose the way on the new first node.
nodes.push(nodes[0]);
} else {
nodes = _.without(way.nodes, nodeId);
}
return graph.replace(way.update({nodes: nodes}));
};
};

View File

@@ -65,7 +65,8 @@ iD.actions.ReverseWay = function(wayId) {
graph.parentRelations(way).forEach(function (relation) {
relation.members.forEach(function (member, index) {
if (member.id === way.id && (role = {forward: 'backward', backward: 'forward'}[member.role])) {
graph = iD.actions.UpdateRelationMember(relation.id, {role: role}, index)(graph);
relation = relation.updateMember({role: role}, index);
graph = graph.replace(relation);
}
});
});

View File

@@ -37,11 +37,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) {
if (relation.isRestriction()) {
var via = relation.memberByRole('via');
if (via && newWay.contains(via.id)) {
graph = iD.actions.UpdateRelationMember(
relation.id,
{id: newWay.id},
relation.memberById(way.id).index
)(graph);
relation = relation.updateMember({id: newWay.id}, relation.memberById(way.id).index);
graph = graph.replace(relation);
}
} else {
var role = relation.memberById(way.id).role,
@@ -55,11 +52,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) {
}
}
graph = iD.actions.AddRelationMember(
relation.id,
{id: newWay.id, type: 'way', role: role},
i <= j ? i + 1 : i
)(graph);
relation = relation.addMember({id: newWay.id, type: 'way', role: role}, i <= j ? i + 1 : i);
graph = graph.replace(relation);
}
});

View File

@@ -20,13 +20,10 @@ iD.actions.UnjoinNode = function(nodeId, newNodeId) {
return;
var index = parent.nodes.indexOf(nodeId),
newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags}),
nodes = parent.nodes.slice();
nodes.splice(index, 1, newNode.id);
newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags});
graph = graph.replace(newNode);
graph = graph.replace(parent.update({nodes: nodes}));
graph = graph.replace(parent.updateNode(newNode.id, index));
});
return graph;

View File

@@ -1,9 +0,0 @@
iD.actions.UpdateRelationMember = function(relationId, properties, index) {
return function(graph) {
var relation = graph.entity(relationId),
members = relation.members.slice();
members.splice(index, 1, _.extend({}, members[index], properties));
return graph.replace(relation.update({members: members}));
};
};

View File

@@ -17,5 +17,9 @@ _.extend(iD.Node.prototype, {
geometry: function() {
return this._poi ? 'point' : 'vertex';
},
move: function(loc) {
return this.update({loc: loc});
}
});

View File

@@ -48,6 +48,23 @@ _.extend(iD.Relation.prototype, {
}
},
addMember: function(member, index) {
var members = this.members.slice();
members.splice(index === undefined ? members.length : index, 0, member);
return this.update({members: members});
},
updateMember: function(member, index) {
var members = this.members.slice();
members.splice(index, 1, _.extend({}, members[index], member));
return this.update({members: members});
},
removeMember: function(id) {
var members = _.reject(this.members, function(m) { return m.id === id; });
return this.update({members: members});
},
isRestriction: function() {
return !!(this.tags.type && this.tags.type.match(/^restriction:?/));
},

View File

@@ -60,5 +60,28 @@ _.extend(iD.Way.prototype, {
geometry: function() {
return this.isArea() ? 'area' : 'line';
},
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});
},
removeNode: function(id) {
var nodes = _.without(this.nodes, id);
// Preserve circularity
if (this.nodes.length > 1 && this.first() === id && this.last() === id) {
nodes.push(nodes[0]);
}
return this.update({nodes: nodes});
}
});

View File

@@ -66,7 +66,6 @@
<script src='../js/id/actions.js'></script>
<script src='../js/id/actions/add_node.js'></script>
<script src="../js/id/actions/add_relation_member.js"></script>
<script src='../js/id/actions/add_way.js'></script>
<script src='../js/id/actions/add_way_node.js'></script>
<script src='../js/id/actions/change_entity_tags.js'></script>
@@ -75,12 +74,9 @@
<script src='../js/id/actions/move_node.js'></script>
<script src='../js/id/actions/move_way.js'></script>
<script src='../js/id/actions/noop.js'></script>
<script src='../js/id/actions/remove_relation_member.js'></script>
<script src='../js/id/actions/remove_way_node.js'></script>
<script src='../js/id/actions/reverse_way.js'></script>
<script src='../js/id/actions/split_way.js'></script>
<script src='../js/id/actions/unjoin_node.js'></script>
<script src='../js/id/actions/update_relation_member.js'></script>
<script src='../js/id/behavior.js'></script>
<script src='../js/id/behavior/drag.js'></script>
@@ -128,21 +124,16 @@
<script src="spec/lib/d3.keybinding.js"></script>
<script src="spec/actions/add_node.js"></script>
<script src="spec/actions/add_relation_member.js"></script>
<script src="spec/actions/add_way.js"></script>
<script src="spec/actions/add_way_node.js"></script>
<script src="spec/actions/change_entity_tags.js"></script>
<script src="spec/actions/delete_node.js"></script>
<script src="spec/actions/delete_way.js"></script>
<script src="spec/actions/move_node.js"></script>
<script src="spec/actions/move_way.js"></script>
<script src="spec/actions/noop.js"></script>
<script src="spec/actions/remove_way_node.js"></script>
<script src="spec/actions/remove_relation_member.js"></script>
<script src="spec/actions/reverse_way.js"></script>
<script src="spec/actions/split_way.js"></script>
<script src='spec/actions/unjoin_node.js'></script>
<script src="spec/actions/update_relation_member.js"></script>
<script src="spec/behavior/hover.js"></script>

View File

@@ -33,7 +33,6 @@
<script src="spec/lib/d3.keybinding.js"></script>
<script src="spec/actions/add_node.js"></script>
<script src="spec/actions/add_relation_member.js"></script>
<script src="spec/actions/add_way.js"></script>
<script src="spec/actions/add_way_node.js"></script>
<script src="spec/actions/change_entity_tags.js"></script>
@@ -42,12 +41,9 @@
<script src="spec/actions/move_node.js"></script>
<script src="spec/actions/move_way.js"></script>
<script src="spec/actions/noop.js"></script>
<script src="spec/actions/remove_way_node.js"></script>
<script src="spec/actions/remove_relation_member.js"></script>
<script src="spec/actions/reverse_way.js"></script>
<script src="spec/actions/split_way.js"></script>
<script src='spec/actions/unjoin_node.js'></script>
<script src="spec/actions/update_relation_member.js"></script>
<script src="spec/behavior/hover.js"></script>

View File

@@ -1,37 +0,0 @@
describe("iD.actions.AddRelationMember", function () {
it("adds a member at the end of the relation", function () {
var relation = iD.Relation(),
graph = iD.Graph([relation]);
graph = iD.actions.AddRelationMember(relation.id, {id: '1'})(graph);
expect(graph.entity(relation.id).members).to.eql([{id: '1'}]);
});
it("adds a member at index 0", function () {
var relation = iD.Relation({members: [{id: '1'}]}),
graph = iD.Graph([relation]);
graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, 0)(graph);
expect(graph.entity(relation.id).members).to.eql([{id: '2'}, {id: '1'}]);
});
it("adds a member at a positive index", function () {
var relation = iD.Relation({members: [{id: '1'}, {id: '3'}]}),
graph = iD.Graph([relation]);
graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, 1)(graph);
expect(graph.entity(relation.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
});
it("adds a member at a negative index", function () {
var relation = iD.Relation({members: [{id: '1'}, {id: '3'}]}),
graph = iD.Graph([relation]);
graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, -1)(graph);
expect(graph.entity(relation.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
});
});

View File

@@ -1,29 +0,0 @@
describe("iD.actions.AddWayNode", function () {
it("adds a node to the end of a way", function () {
var way = iD.Way(),
node = iD.Node({id: "n1"}),
graph = iD.actions.AddWayNode(way.id, node.id)(iD.Graph([way, node]));
expect(graph.entity(way.id).nodes).to.eql(["n1"]);
});
it("adds a node to a way at index 0", function () {
var way = iD.Way({nodes: ["n1", "n3"]}),
node = iD.Node({id: "n2"}),
graph = iD.actions.AddWayNode(way.id, node.id, 0)(iD.Graph([way, node]));
expect(graph.entity(way.id).nodes).to.eql(["n2", "n1", "n3"]);
});
it("adds a node to a way at a positive index", function () {
var way = iD.Way({nodes: ["n1", "n3"]}),
node = iD.Node({id: "n2"}),
graph = iD.actions.AddWayNode(way.id, node.id, 1)(iD.Graph([way, node]));
expect(graph.entity(way.id).nodes).to.eql(["n1", "n2", "n3"]);
});
it("adds a node to a way at a negative index", function () {
var way = iD.Way({nodes: ["n1", "n3"]}),
node = iD.Node({id: "n2"}),
graph = iD.actions.AddWayNode(way.id, node.id, -1)(iD.Graph([way, node]));
expect(graph.entity(way.id).nodes).to.eql(["n1", "n2", "n3"]);
});
});

View File

@@ -1,8 +0,0 @@
describe("iD.actions.RemoveRelationMember", function () {
it("removes a member from a relation", function () {
var node = iD.Node(),
relation = iD.Way({members: [{ id: node.id }]}),
graph = iD.actions.RemoveRelationMember(relation.id, node.id)(iD.Graph([node, relation]));
expect(graph.entity(relation.id).members).to.eql([]);
});
});

View File

@@ -1,8 +0,0 @@
describe("iD.actions.RemoveWayNode", function () {
it("removes a node from a way", function () {
var node = iD.Node({id: "n1"}),
way = iD.Way({id: "w1", nodes: ["n1"]}),
graph = iD.actions.RemoveWayNode(way.id, node.id)(iD.Graph({n1: node, w1: way}));
expect(graph.entity(way.id).nodes).to.eql([]);
});
});

View File

@@ -1,8 +0,0 @@
describe("iD.actions.UpdateRelationMember", function () {
it("updates the properties of the relation member at the specified index", function () {
var node = iD.Node(),
relation = iD.Relation({members: [{id: node.id, role: 'forward'}]}),
graph = iD.actions.UpdateRelationMember(relation.id, {role: 'backward'}, 0)(iD.Graph([node, relation]));
expect(graph.entity(relation.id).members).to.eql([{id: node.id, role: 'backward'}]);
});
});

View File

@@ -104,6 +104,42 @@ describe('iD.Relation', function () {
});
});
describe("#addMember", function () {
it("adds a member at the end of the relation", function () {
var r = iD.Relation();
expect(r.addMember({id: '1'}).members).to.eql([{id: '1'}]);
});
it("adds a member at index 0", function () {
var r = iD.Relation({members: [{id: '1'}]});
expect(r.addMember({id: '2'}, 0).members).to.eql([{id: '2'}, {id: '1'}]);
});
it("adds a member at a positive index", function () {
var r = iD.Relation({members: [{id: '1'}, {id: '3'}]});
expect(r.addMember({id: '2'}, 1).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
});
it("adds a member at a negative index", function () {
var r = iD.Relation({members: [{id: '1'}, {id: '3'}]});
expect(r.addMember({id: '2'}, -1).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
});
});
describe("#updateMember", function () {
it("updates the properties of the relation member at the specified index", function () {
var r = iD.Relation({members: [{role: 'forward'}]});
expect(r.updateMember({role: 'backward'}, 0).members).to.eql([{role: 'backward'}]);
});
});
describe("#removeMember", function () {
it("removes a member", function () {
var r = iD.Relation({members: [{id: 'a'}]});
expect(r.removeMember('a').members).to.eql([]);
});
});
describe("#multipolygon", function () {
specify("single polygon consisting of a single way", function () {
var a = iD.Node(),

View File

@@ -126,4 +126,52 @@ describe('iD.Way', function() {
expect(iD.Way({tags: { area: 'yes' }}).geometry()).to.equal('area');
});
});
describe("#addNode", function () {
it("adds a node to the end of a way", function () {
var w = iD.Way();
expect(w.addNode('a').nodes).to.eql(['a']);
});
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']);
});
it("adds a node to a way at a positive index", function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('c', 1).nodes).to.eql(['a', 'c', 'b']);
});
it("adds a node to a way at a negative index", function () {
var w = iD.Way({nodes: ['a', 'b']});
expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']);
});
});
describe("#updateNode", function () {
it("updates the node id at the specified index", function () {
var w = iD.Way({nodes: ['a', 'b', 'c']});
expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']);
});
});
describe("#removeNode", function () {
it("removes the node", function () {
var a = iD.Node({id: 'a'}),
w = iD.Way({nodes: ['a']});
expect(w.removeNode('a').nodes).to.eql([]);
});
it("preserves circularity", function () {
var a = iD.Node({id: 'a'}),
b = iD.Node({id: 'b'}),
c = iD.Node({id: 'c'}),
d = iD.Node({id: 'd'}),
w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'a']});
expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']);
});
});
});