Skip to content

Commit

Permalink
perf: Merge pull request #70 from pelias/remove-set-lat-set-lon
Browse files Browse the repository at this point in the history
perf: removed setLat/setLon/getLat/getLon, moved tests to centroid

BREAKING CHANGE: removes unneeded functions
  • Loading branch information
trescube authored Jul 10, 2017
2 parents fcfbbfe + cd40cf7 commit 9cd9847
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 144 deletions.
46 changes: 14 additions & 32 deletions Document.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,36 +374,6 @@ Document.prototype.getPopularity = function(){
return this.popularity;
};

// longitude
Document.prototype.setLon = function( value ){

value = transform.floatify(6, value);
validate.type('number', value);
validate.geo('longitude', value);

this.center_point.lon = value;
return this;
};

Document.prototype.getLon = function(){
return this.center_point.lon;
};

// latitude
Document.prototype.setLat = function( value ){

value = transform.floatify(6, value);
validate.type('number', value);
validate.geo('latitude', value);

this.center_point.lat = value;
return this;
};

Document.prototype.getLat = function(){
return this.center_point.lat;
};

// categories
Document.prototype.addCategory = function( value ){

Expand Down Expand Up @@ -432,8 +402,20 @@ Document.prototype.removeCategory = function( value ){
// centroid
Document.prototype.setCentroid = function( centroid ){
centroid = centroid || {};
this.setLon.call( this, centroid.lon );
this.setLat.call( this, centroid.lat );

centroid.lon = transform.floatify(6, centroid.lon);
validate.type('number', centroid.lon);
validate.geo('longitude', centroid.lon);

centroid.lat = transform.floatify(6, centroid.lat);
validate.type('number', centroid.lat);
validate.geo('latitude', centroid.lat);

// copy the lat/lon values instead of assigning the object or the object
// could be changed outside
this.center_point.lon = centroid.lon;
this.center_point.lat = centroid.lat;

return this;
};

Expand Down
55 changes: 55 additions & 0 deletions test/document/centroid.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@ module.exports.tests.getCentroid = function(test) {
module.exports.tests.setCentroid = function(test) {
test('setCentroid', function(t) {
var doc = new Document('mysource','mylayer','myid');

t.equal(doc.setCentroid({ lon: 1, lat: 2 }), doc, 'chainable');
t.deepEqual(doc.center_point, { lat: 2, lon: 1 }, 'setter works');
t.equal(doc.setCentroid({lon:'1.1',lat:'1.2'}), doc, 'chainable');
t.deepEqual(doc.center_point, { lat: 1.2, lon: 1.1 }, 'setter works');
t.end();

});

test('setCentroid - validate', function(t) {
var doc = new Document('mysource','mylayer','myid');
t.throws( doc.setCentroid.bind(doc), null, 'invalid args' );
Expand All @@ -32,6 +35,58 @@ module.exports.tests.setCentroid = function(test) {
t.throws( doc.setCentroid.bind(doc,{}), null, 'missing lat/lon' );
t.end();
});

test('string lat and lon properties in parameters should be transformed and fixed precision', (t) => {
const doc = new Document('mysource','mylayer','myid');

doc.setCentroid({ lat: '12.3456789', lon: '-12.3456789'});

t.deepEquals(doc.getCentroid(), { lat: 12.345679, lon: -12.345679 });
t.end();

});

test('extremes should be accepted', (t) => {
const doc = new Document('mysource','mylayer','myid');

doc.setCentroid({ lat: -90, lon: -180 });
t.deepEquals(doc.getCentroid(), { lat: -90, lon: -180 }, 'accepts mins');

doc.setCentroid({ lat: 0, lon: 0 });
t.deepEquals(doc.getCentroid(), { lat: 0, lon: 0 }, 'accepts zeroes');

doc.setCentroid({ lat: 90, lon: 180 });
t.deepEquals(doc.getCentroid(), { lat: 90, lon: 180 }, 'accepts maxs');

t.end();

});

test('latitude and longitude values outside of acceptable boundes should be rejected', (t) => {
const doc = new Document('mysource','mylayer','myid');

t.throws( doc.setCentroid.bind(doc, {lat: 90.000001, lon: 0 }), null, 'invalid range' );
t.throws( doc.setCentroid.bind(doc, {lat: -90.000001, lon: 0 }), null, 'invalid range' );
t.throws( doc.setCentroid.bind(doc, {lat: 0, lon: 180.000001 }), null, 'invalid range' );
t.throws( doc.setCentroid.bind(doc, {lat: 0, lon: -180.000001 }), null, 'invalid range' );
t.end();

});

test('modifying parameter object after setCentroid shows that source was value-copied', (t) => {
const doc = new Document('mysource','mylayer','myid');

const centroid = { lat: 12.121212, lon: 21.212121 };
doc.setCentroid(centroid);

centroid.lat = 13.131313;
centroid.lon = 31.313131;

t.deepEquals(doc.getCentroid(), { lat: 12.121212, lon: 21.212121 }, 'should not have changed');
t.end();

});

};

module.exports.all = function (tape, common) {
Expand Down
111 changes: 0 additions & 111 deletions test/document/latlon.js

This file was deleted.

1 change: 0 additions & 1 deletion test/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ var tests = [
require('./document/centroid.js'),
require('./document/gid.js'),
require('./document/id.js'),
require('./document/latlon.js'),
require('./document/meta.js'),
require('./document/popularity.js'),
require('./document/population.js'),
Expand Down

0 comments on commit 9cd9847

Please sign in to comment.