mapmeld / iD

Developing GeoServices tool for OSM iD Editor
http://mapmeld.com/iD
ISC License
7 stars 1 forks source link

avoid attaching to 'window' #32

Closed jgravois closed 6 years ago

jgravois commented 7 years ago

this is just a code smell thing.

modules/svg/geoservice.js#L17-L24

// dictionary matching geo-properties to OpenStreetMap tags 1:1
window.layerImports = {};
window.layerChecked = {};

// prevent re-downloading and re-adding the same feature
window.knownObjectIds = {};

// keeping track of added OSM entities
window.importedEntities = [];

it'd be preferable to wrap these arrays and objects as module level globals instead of attaching them directly to window.

mapmeld commented 7 years ago

agreed... I added these early on, before I had a handle on how this would be structured. I'll see if they can be part of the module

jgravois commented 7 years ago

i took a look at this this afternoon, the bulk of the references to 'Window' occur within the _map call inside drawGeoService.

it seems like the most best thing to do would be to pass the appropriate context through there as the third parameter, but this didn't work the way i hoped.


var app = {
  knownObjectIds: {}
};

// ...

_.map(geojson.features || [], function(d) {
  // app.knownObjectIds etc.
}, app);
jgravois commented 6 years ago

nice work @mapmeld!