From 735f490f56fa5b7842509d938e0ae0310c6f6c42 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 20 Feb 2013 00:02:50 -0700 Subject: [PATCH] Treating geometry as just another attribute The first set geometry is considered the default. As an added bonus, we're back to a single argument constructor. Later, we could allow a schema to be set. This would be done before setting values (calling constructor with no args). --- examples/vector-layer.js | 12 +- src/ol/feature.js | 36 ++++- src/ol/io/geojson.js | 7 +- test/spec/ol/feature.test.js | 161 +++++++++++++++++++++++ test/spec/ol/filter/extentfilter.test.js | 4 +- test/spec/ol/source/vectorsource.test.js | 35 +++-- test/spec/ol/style/line.test.js | 2 +- test/spec/ol/style/polygon.test.js | 2 +- test/spec/ol/style/shape.test.js | 2 +- 9 files changed, 233 insertions(+), 28 deletions(-) create mode 100644 test/spec/ol/feature.test.js diff --git a/examples/vector-layer.js b/examples/vector-layer.js index cad588b797..ef568efd69 100644 --- a/examples/vector-layer.js +++ b/examples/vector-layer.js @@ -22,11 +22,13 @@ var source = new ol.source.Vector({ }); source.addFeatures([ - new ol.Feature( - new ol.geom.LineString([[-10000000, -10000000], [10000000, 10000000]])), - new ol.Feature( - new ol.geom.LineString([[-10000000, 10000000], [10000000, -10000000]])), - new ol.Feature(new ol.geom.Point([-10000000, 5000000])) + new ol.Feature({ + g: new ol.geom.LineString([[-10000000, -10000000], [10000000, 10000000]]) + }), + new ol.Feature({ + g: new ol.geom.LineString([[-10000000, 10000000], [10000000, -10000000]]) + }, + new ol.Feature({g: new ol.geom.Point([-10000000, 5000000])}) ]); var vector = new ol.layer.Vector({ diff --git a/src/ol/feature.js b/src/ol/feature.js index f4d46ac08a..f12deb2366 100644 --- a/src/ol/feature.js +++ b/src/ol/feature.js @@ -8,18 +8,17 @@ goog.require('ol.geom.Geometry'); /** * @constructor * @extends {ol.Object} - * @param {ol.geom.Geometry=} opt_geometry Geometry. * @param {Object=} opt_values Attributes. */ -ol.Feature = function(opt_geometry, opt_values) { +ol.Feature = function(opt_values) { goog.base(this, opt_values); /** + * @type {string|undefined} * @private - * @type {ol.geom.Geometry} */ - this.geometry_ = goog.isDef(opt_geometry) ? opt_geometry : null; + this.geometryName_; }; goog.inherits(ol.Feature, ol.Object); @@ -47,7 +46,22 @@ ol.Feature.prototype.getAttributes = function() { * @return {ol.geom.Geometry} The geometry (or null if none). */ ol.Feature.prototype.getGeometry = function() { - return this.geometry_; + return goog.isDef(this.geometryName_) ? + /** @type {ol.geom.Geometry} */ (this.get(this.geometryName_)) : + null; +}; + + +/** + * @inheritDoc + * @param {string} key Key. + * @param {*} value Value. + */ +ol.Feature.prototype.set = function(key, value) { + if (!goog.isDef(this.geometryName_) && (value instanceof ol.geom.Geometry)) { + this.geometryName_ = key; + } + goog.base(this, 'set', key, value); }; @@ -55,5 +69,15 @@ ol.Feature.prototype.getGeometry = function() { * @param {ol.geom.Geometry} geometry The geometry. */ ol.Feature.prototype.setGeometry = function(geometry) { - this.geometry_ = geometry; + if (!goog.isDef(this.geometryName_)) { + this.geometryName_ = ol.Feature.DEFAULT_GEOMETRY; + } + this.set(this.geometryName_, geometry); }; + + +/** + * @const + * @type {string} + */ +ol.Feature.DEFAULT_GEOMETRY = 'geometry'; diff --git a/src/ol/io/geojson.js b/src/ol/io/geojson.js index 2dd708e641..bfd8bd8250 100644 --- a/src/ol/io/geojson.js +++ b/src/ol/io/geojson.js @@ -84,12 +84,15 @@ ol.io.geojson.parse_ = function(json) { */ ol.io.geojson.parseFeature_ = function(json) { var geomJson = json.geometry, - geometry; + geometry = null; if (geomJson) { geometry = /** @type {ol.geom.Geometry} */ (ol.io.geojson.parse_( /** @type {GeoJSONGeometry} */ (geomJson))); } - return new ol.Feature(geometry, json.properties); + var feature = new ol.Feature(); + feature.setGeometry(geometry); + feature.setValues(json.properties); + return feature; }; diff --git a/test/spec/ol/feature.test.js b/test/spec/ol/feature.test.js new file mode 100644 index 0000000000..20cf3579ce --- /dev/null +++ b/test/spec/ol/feature.test.js @@ -0,0 +1,161 @@ +goog.provide('ol.test.Feature'); + +describe('ol.Feature', function() { + + describe('constructor', function() { + + it('creates a new feature', function() { + var feature = new ol.Feature(); + expect(feature).toBeA(ol.Feature); + }); + + it('takes attribute values', function() { + var feature = new ol.Feature({ + foo: 'bar' + }); + expect(feature.get('foo')).toBe('bar'); + }); + + it('will set the default geometry', function() { + var feature = new ol.Feature({ + loc: new ol.geom.Point([10, 20]), + foo: 'bar' + }); + var geometry = feature.getGeometry(); + expect(geometry).toBeA(ol.geom.Point); + expect(feature.get('loc')).toBe(geometry); + }); + + }); + + describe('#get()', function() { + + it('returns values set at construction', function() { + var feature = new ol.Feature({ + a: 'first', + b: 'second' + }); + expect(feature.get('a')).toBe('first'); + expect(feature.get('b')).toBe('second'); + }); + + it('returns undefined for unset attributes', function() { + var feature = new ol.Feature(); + expect(feature.get('a')).toBeUndefined(); + }); + + it('returns values set by set', function() { + var feature = new ol.Feature(); + feature.set('a', 'b'); + expect(feature.get('a')).toBe('b'); + }); + + }); + + describe('#getGeometry()', function() { + + var point = new ol.geom.Point([15, 30]); + + it('returns null for no geometry', function() { + var feature = new ol.Feature(); + expect(feature.getGeometry()).toBeNull(); + }); + + it('gets the geometry set at construction', function() { + var feature = new ol.Feature({ + geom: point + }); + expect(feature.getGeometry()).toBe(point); + }); + + it('gets any geometry set by setGeometry', function() { + var feature = new ol.Feature(); + feature.setGeometry(point); + expect(feature.getGeometry()).toBe(point); + + var point2 = new ol.geom.Point([1, 2]); + feature.setGeometry(point2); + expect(feature.getGeometry()).toBe(point2); + }); + + it('gets the first geometry set by set', function() { + var feature = new ol.Feature(); + feature.set('foo', point); + expect(feature.getGeometry()).toBe(point); + + feature.set('bar', new ol.geom.Point([1, 2])); + expect(feature.getGeometry()).toBe(point); + }); + + }); + + describe('#set()', function() { + + it('sets values', function() { + var feature = new ol.Feature({ + a: 'first', + b: 'second' + }); + feature.set('a', 'new'); + expect(feature.get('a')).toBe('new'); + }); + + it('can be used to set the geometry', function() { + var point = new ol.geom.Point([3, 4]); + var feature = new ol.Feature({ + loc: new ol.geom.Point([1, 2]) + }); + feature.set('loc', point); + expect(feature.get('loc')).toBe(point); + expect(feature.getGeometry()).toBe(point); + }); + + }); + + describe('#setGeometry()', function() { + + var point = new ol.geom.Point([15, 30]); + + it('sets the default geometry', function() { + var feature = new ol.Feature(); + feature.setGeometry(point); + expect(feature.get(ol.Feature.DEFAULT_GEOMETRY)).toBe(point); + }); + + it('replaces previous default geometry', function() { + var feature = new ol.Feature({ + geom: point + }); + expect(feature.getGeometry()).toBe(point); + + var point2 = new ol.geom.Point([1, 2]); + feature.setGeometry(point2); + expect(feature.getGeometry()).toBe(point2); + }); + + it('gets any geometry set by setGeometry', function() { + var feature = new ol.Feature(); + feature.setGeometry(point); + expect(feature.getGeometry()).toBe(point); + + var point2 = new ol.geom.Point([1, 2]); + feature.setGeometry(point2); + expect(feature.getGeometry()).toBe(point2); + }); + + it('gets the first geometry set by set', function() { + var feature = new ol.Feature(); + feature.set('foo', point); + expect(feature.getGeometry()).toBe(point); + + feature.set('bar', new ol.geom.Point([1, 2])); + expect(feature.getGeometry()).toBe(point); + }); + + }); + +}); + + +goog.require('ol.Feature'); +goog.require('ol.geom.Point'); diff --git a/test/spec/ol/filter/extentfilter.test.js b/test/spec/ol/filter/extentfilter.test.js index 68e1a0a2e7..f4040d72f8 100644 --- a/test/spec/ol/filter/extentfilter.test.js +++ b/test/spec/ol/filter/extentfilter.test.js @@ -21,9 +21,9 @@ describe('ol.filter.Extent', function() { describe('#evaluate()', function() { it('returns true if a feature intersects, false if not', function() { - expect(filter.evaluate(new ol.Feature(new ol.geom.Point([44, 89])))) + expect(filter.evaluate(new ol.Feature({g: new ol.geom.Point([44, 89])}))) .toBe(true); - expect(filter.evaluate(new ol.Feature(new ol.geom.Point([46, 91])))) + expect(filter.evaluate(new ol.Feature({g: new ol.geom.Point([46, 91])}))) .toBe(false); }); diff --git a/test/spec/ol/source/vectorsource.test.js b/test/spec/ol/source/vectorsource.test.js index f3ac91aeda..8bf8df1f43 100644 --- a/test/spec/ol/source/vectorsource.test.js +++ b/test/spec/ol/source/vectorsource.test.js @@ -22,14 +22,30 @@ describe('ol.source.Vector', function() { beforeEach(function() { features = [ - new ol.Feature(new ol.geom.Point([16.0, 48.0])), - new ol.Feature(new ol.geom.Point([16.1, 48.1])), - new ol.Feature(new ol.geom.Point([16.2, 48.2])), - new ol.Feature(new ol.geom.Point([16.3, 48.3])), - new ol.Feature(new ol.geom.LineString([[16.4, 48.4], [16.5, 48.5]])), - new ol.Feature(new ol.geom.LineString([[16.6, 48.6], [16.7, 48.7]])), - new ol.Feature(new ol.geom.LineString([[16.8, 48.8], [16.9, 48.9]])), - new ol.Feature(new ol.geom.LineString([[17.0, 49.0], [17.1, 49.1]])) + new ol.Feature({ + g: new ol.geom.Point([16.0, 48.0]) + }), + new ol.Feature({ + g: new ol.geom.Point([16.1, 48.1]) + }), + new ol.Feature({ + g: new ol.geom.Point([16.2, 48.2]) + }), + new ol.Feature({ + g: new ol.geom.Point([16.3, 48.3]) + }), + new ol.Feature({ + g: new ol.geom.LineString([[16.4, 48.4], [16.5, 48.5]]) + }), + new ol.Feature({ + g: new ol.geom.LineString([[16.6, 48.6], [16.7, 48.7]]) + }), + new ol.Feature({ + g: new ol.geom.LineString([[16.8, 48.8], [16.9, 48.9]]) + }), + new ol.Feature({ + g: new ol.geom.LineString([[17.0, 49.0], [17.1, 49.1]]) + }) ]; vectorSource = new ol.source.Vector({ projection: ol.Projection.getFromCode('EPSG:4326') @@ -71,8 +87,7 @@ describe('ol.source.Vector', function() { expect(subset2.length).toEqual(0); }); - it('can handle any query using the filter\'s evaluate function', - function() { + it('can handle query using the filter\'s evaluate function', function() { var filter = new ol.filter.Logical([geomFilter, extentFilter], ol.filter.LogicalOperator.OR); spyOn(filter, 'evaluate').andCallThrough(); diff --git a/test/spec/ol/style/line.test.js b/test/spec/ol/style/line.test.js index 78fc4b1a16..aac6cebe4a 100644 --- a/test/spec/ol/style/line.test.js +++ b/test/spec/ol/style/line.test.js @@ -30,7 +30,7 @@ describe('ol.style.Line', function() { strokeWidth: ol.Expression('widthAttr') }); - var feature = new ol.Feature(undefined, { + var feature = new ol.Feature({ value: 42, widthAttr: 1.5 }); diff --git a/test/spec/ol/style/polygon.test.js b/test/spec/ol/style/polygon.test.js index 436ba77cd4..b4c4722a9f 100644 --- a/test/spec/ol/style/polygon.test.js +++ b/test/spec/ol/style/polygon.test.js @@ -30,7 +30,7 @@ describe('ol.style.Polygon', function() { fillStyle: new ol.Expression('fillAttr') }); - var feature = new ol.Feature(undefined, { + var feature = new ol.Feature({ value: 42, fillAttr: '#ff0000' }); diff --git a/test/spec/ol/style/shape.test.js b/test/spec/ol/style/shape.test.js index acf0996e90..26405624e6 100644 --- a/test/spec/ol/style/shape.test.js +++ b/test/spec/ol/style/shape.test.js @@ -30,7 +30,7 @@ describe('ol.style.Shape', function() { opacity: new ol.Expression('opacityAttr') }); - var feature = new ol.Feature(undefined, { + var feature = new ol.Feature({ sizeAttr: 42, opacityAttr: 0.4 });