Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multi-parents ? #129

Open
Joxit opened this issue Mar 22, 2020 · 0 comments
Open

Support for multi-parents ? #129

Joxit opened this issue Mar 22, 2020 · 0 comments

Comments

@Joxit
Copy link
Member

Joxit commented Mar 22, 2020

I was working on this project and I saw this message :

model/Document.js

Lines 345 to 368 in 37c4330

/**
note: the rationale for setting this field as 'null' instead of 'undefined'
is so that each of the fields 'line-up'.
== if you add a parent property with no abbreviation, as such:
addParent( 'region', 'foobar', '1' )
doc:
parent.region = [ 'foobar' ]
parent.region_id = [ '1' ]
parent.region_a = [ null ]
== and then you add another parent property such as:
addParent( 'region', 'bingobango', '2', 'bingo' )
doc:
parent.region = [ 'foobar', 'bingobango' ]
parent.region_id = [ '1', '2' ]
parent.region_a = [ null, 'bingo' ]
== you can now be sure that the abbreviation 'bingo' belongs to '2' and not '1'.
**/

I thought it was a pretty cool feature but when I saw this piece of code I thought it wasn't really going to work ...

model/Document.js

Lines 321 to 332 in 37c4330

var add = function( prop, value ){
// create new parent array if required
if( !this.parent.hasOwnProperty( prop ) ){
this.parent[ prop ] = [];
}
// add value to array if not already present
if( -1 === this.parent[prop].indexOf(value) ){
this.parent[prop].push(value);
}
}.bind(this);

This does not really work as expected. For example in this example, the second London and LD will not be added and this can be an wired 🤔. (osm:locality:4004 with no name)

  var doc = new Document( 'geoname', 'venue', 1003 )
      .setCentroid({ lon: 0.5, lat: 50.1 })
      .addParent( 'locality', 'London', '3003', 'LD', 'whosonfirst' )
      .addParent( 'locality', 'London', '4004', 'LD', 'osm' )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant