mapbox / leaflet-omnivore

universal format parser for Leaflet & Mapbox.js
https://www.mapbox.com/mapbox.js/example/v1.0.0/omnivore-gpx/
Other
651 stars 126 forks source link

TopoJSON parsing #93

Closed bhaskarvk closed 8 years ago

bhaskarvk commented 8 years ago

Hi this is not an issue but an enhancement request.

function topojsonParse(data, options, layer) {
    var o = typeof data === 'string' ?
        JSON.parse(data) : data;
    layer = layer || L.geoJson();
    for (var i in o.objects) {
        var ft = topojson.feature(o, o.objects[i]);
        if (ft.features) addData(layer, ft.features);
        else addData(layer, ft);
    }
    return layer;

L.GeoJSON already has an explicit check to determine whether the passed in GeoJSON is a FeatureCollection object or an array of Features, so instead of

        if (ft.features) addData(layer, ft.features);
        else addData(layer, ft);

It is convenient to just call addData(layer, ft). That way it's much straightforward to check for valid GeoJSON in a custom setGeoJSON() call, otherwise we need to repeat the same code in L.GeoJSON's addData which checks for Array or FeatureCollection object. Removing the if/else part and just calling addData(layer, ft) won't break anything but will make handing much simpler in a custom setGeoJSON.

Thanks

bhaskarvk commented 8 years ago

Not really needed, the check is simple enough.