From fdd49233b14c8c1d8c22d48161e3f50687ef17f9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 24 Apr 2013 13:37:16 -0700 Subject: [PATCH] Avoid recomputing geometry in an inner loop --- js/id/presets.js | 6 +++--- js/id/presets/category.js | 4 ++-- js/id/presets/collection.js | 6 +++--- js/id/presets/preset.js | 4 ++-- js/id/ui/preset_grid.js | 2 +- test/spec/presets/category.js | 4 ++-- test/spec/presets/collection.js | 4 ++-- test/spec/presets/preset.js | 9 +++++---- 8 files changed, 20 insertions(+), 19 deletions(-) diff --git a/js/id/presets.js b/js/id/presets.js index 588976ed4..29e4f7ba3 100644 --- a/js/id/presets.js +++ b/js/id/presets.js @@ -57,9 +57,9 @@ iD.presets = function(context) { }; all.defaults = function(entity, n) { - var rec = recent.matchGeometry(entity, context.graph()).collection.slice(0, 4), - def = _.uniq(rec.concat(defaults[entity.geometry(context.graph())].collection)).slice(0, n - 1), - geometry = entity.geometry(context.graph()); + var geometry = entity.geometry(context.graph()), + rec = recent.matchGeometry(geometry).collection.slice(0, 4), + def = _.uniq(rec.concat(defaults[geometry].collection)).slice(0, n - 1); return iD.presets.Collection(_.unique(rec.concat(def).concat(geometry === 'area' ? other_area : other))); }; diff --git a/js/id/presets/category.js b/js/id/presets/category.js index c3dbfa159..5367f9407 100644 --- a/js/id/presets/category.js +++ b/js/id/presets/category.js @@ -7,8 +7,8 @@ iD.presets.Category = function(id, category, all) { return all.item(id); })); - category.matchGeometry = function(entity, resolver) { - return category.geometry.indexOf(entity.geometry(resolver)) >= 0; + category.matchGeometry = function(geometry) { + return category.geometry.indexOf(geometry) >= 0; }; category.matchTags = function() { return false; }; diff --git a/js/id/presets/collection.js b/js/id/presets/collection.js index a43a1ef26..d21916a21 100644 --- a/js/id/presets/collection.js +++ b/js/id/presets/collection.js @@ -11,12 +11,12 @@ iD.presets.Collection = function(collection) { }, match: function(entity, resolver) { - return presets.matchGeometry(entity, resolver).matchTags(entity); + return presets.matchGeometry(entity.geometry(resolver)).matchTags(entity); }, - matchGeometry: function(entity, resolver) { + matchGeometry: function(geometry) { return iD.presets.Collection(collection.filter(function(d) { - return d.matchGeometry(entity, resolver); + return d.matchGeometry(geometry); })); }, diff --git a/js/id/presets/preset.js b/js/id/presets/preset.js index 8ebfb7297..76df447f3 100644 --- a/js/id/presets/preset.js +++ b/js/id/presets/preset.js @@ -8,8 +8,8 @@ iD.presets.Preset = function(id, preset, fields) { return fields[f]; } - preset.matchGeometry = function(entity, resolver) { - return preset.geometry.indexOf(entity.geometry(resolver)) >= 0; + preset.matchGeometry = function(geometry) { + return preset.geometry.indexOf(geometry) >= 0; }; preset.matchTags = function(entity) { diff --git a/js/id/ui/preset_grid.js b/js/id/ui/preset_grid.js index e4c458fa0..ca6218bd5 100644 --- a/js/id/ui/preset_grid.js +++ b/js/id/ui/preset_grid.js @@ -9,7 +9,7 @@ iD.ui.PresetGrid = function(context, entity) { selection.html(''); - presets = context.presets().matchGeometry(entity, context.graph()); + presets = context.presets().matchGeometry(entity.geometry(context.graph())); var messagewrap = selection.append('div') .attr('class', 'header fillL cf'); diff --git a/test/spec/presets/category.js b/test/spec/presets/category.js index 9f22ad441..94dd034f7 100644 --- a/test/spec/presets/category.js +++ b/test/spec/presets/category.js @@ -29,8 +29,8 @@ describe("iD.presets.Category", function() { w = iD.Way(), n = iD.Node(), g = iD.Graph().replace(w); - expect(c.matchGeometry(w, g)).to.eql(true); - expect(c.matchGeometry(n, g)).to.eql(false); + expect(c.matchGeometry('line')).to.eql(true); + expect(c.matchGeometry('point')).to.eql(false); }); }); }); diff --git a/test/spec/presets/collection.js b/test/spec/presets/collection.js index 7e49aefac..8bfb8d416 100644 --- a/test/spec/presets/collection.js +++ b/test/spec/presets/collection.js @@ -31,8 +31,8 @@ describe("iD.presets.Collection", function() { }); describe("#matchGeometry", function() { - it("returns a new collection only containing presets matching an entity's type", function() { - expect(c.matchGeometry(w, g).collection).to.eql([p.other, p.residential]); + it("returns a new collection only containing presets matching a geometry", function() { + expect(c.matchGeometry('line').collection).to.eql([p.other, p.residential]); }); }); diff --git a/test/spec/presets/preset.js b/test/spec/presets/preset.js index 6e8015c9c..56483285e 100644 --- a/test/spec/presets/preset.js +++ b/test/spec/presets/preset.js @@ -40,12 +40,13 @@ describe('iD.presets.Preset', function() { describe('#matchGeometry', function() { var n = iD.Node(); var g = iD.Graph().replace(n); - it("returns false if it doesn't match the entity type", function() { - expect(p['highway/residential'].matchGeometry(n, g)).to.equal(false); + + it("returns false if it doesn't match", function() { + expect(p['highway/residential'].matchGeometry('point')).to.equal(false); }); - it("returns true if it does match the entity type", function() { - expect(p.other.matchGeometry(n, g)).to.equal(true); + it("returns true if it does match", function() { + expect(p.other.matchGeometry('point')).to.equal(true); }); });