I wrote this code for an personal application, just to help me visualize some data related to driving. I have one JSON on my machine, this JSON contains a set of locations (lat, lngs) and every location has a category (integer number).
After I fetch this file I want to use Google's Road API, to snap the points on the road. Unfortunately it only allows 100 points per request. And depending on the "track" I can have usually 400+ points.
At this point, this application is only maintained by me. But I used this as an excuse to re-learn Javascript (including the new stuff only new browsers have).
I used Promises and I was used to the callback hell of before and after reading so much good stuff about Promises I got really frustrated seeing this huge chaining I made.
It's working, can't complain about that. But, looks messy.
document.addEventListener('DOMContentLoaded', function(evt){
'use strict';
const apiKey = '-----------';
let rawMap = new google.maps.Map(document.getElementById('rawMap'), {
center: {lat: 0, lng: 0},
zoom: 10
});
let snappedMap = new google.maps.Map(document.getElementById('snappedMap'), {
center: {lat: 0, lng: 0},
zoom: 10
});
const colors = [
'#FF0000',
'#00FF00',
'#0000FF',
'#FFFF00',
'#00FFFF',
'#FF00FF',
'#000000',
];
fetch('output.json').then(response => response.json()).then(function(points){
for (let i = 0 ; i < points.length - 1 ; ++i) {
let path = new google.maps.Polyline({
path: [points[i], points[i+1]],
strokeColor: colors[points[i]['cat']],
strokeWeight: 5
});
path.setMap(rawMap);
}
let bounds = new google.maps.LatLngBounds();
points.forEach(point => bounds.extend(point));
rawMap.fitBounds(bounds)
snappedMap.fitBounds(bounds);
let requests = [];
for (let i = 0; i < points.length ; i += 100) {
let params = new URLSearchParams();
params.append('interpolate', 'true');
params.append('key', apiKey)
params.append('path', points.slice(i, i + 100).map(point => `${point['lat']},${point['lng']}`).join('|'));
requests.push(fetch(`https://roads.googleapis.com/v1/snapToRoads?${params}`));
}
return Promise.all(requests)
.then(responses => Promise.all(responses.map(response => response.json())))
.then(function(jsons){
for (let i = 0, lastIndex = -1; i < jsons.length ; ++i) {
let snappedPoints = jsons[i].snappedPoints;
for (let snappedPoint of snappedPoints) {
if ('originalIndex' in snappedPoint) {
lastIndex = i * 100 + snappedPoint.originalIndex;
}
snappedPoint['cat'] = points[lastIndex]['cat'];
}
}
return jsons;
});
}).then(function(jsons) {
return jsons.reduce((ret, json) => ret.concat(json.snappedPoints), [])
}).then(function(snappedPoints){
for (let i = 0; i < snappedPoints.length - 1; i++) {
let path = new google.maps.Polyline({
path: [
{
lat: snappedPoints[i].location.latitude,
lng: snappedPoints[i].location.longitude
},
{
lat: snappedPoints[i+1].location.latitude,
lng: snappedPoints[i+1].location.longitude
}
],
strokeWeight:5,
strokeColor: colors[snappedPoints[i]['cat']]
});
path.setMap(snappedMap);
}
}).catch(function(error){
console.log(error);
});
});
2 Answers 2
- Divide processing in simple functions, then chain them in linear fashion.
- Use literal
.property
or['property']
consistently, don't mix them as it's the same in JS - Concatenate arrays in one operation via
Array.prototype.concat.apply([], arrayOfArrays)
instead ofreduce
that creates intermediate arrays (it's slow and memory-hogging). However, if the number of elements may exceed 32k (the max number of arguments in some browsers) it's safer toconcat
in chunks of 10k by using a loop and.slice
.
// your init part
let points;
const getJSON = (response) => response.json();
const drawPoints = (json) => {
// will be used later for snapping to roads
points = json;
for (let i = 0; i < points.length - 1; ++i) {
let path = new google.maps.Polyline({
path: [points[i], points[i + 1]],
strokeColor: colors[points[i].cat],
strokeWeight: 5
});
path.setMap(rawMap);
}
// fit map bounds to the points
let bounds = new google.maps.LatLngBounds();
points.forEach(point => bounds.extend(point));
rawMap.fitBounds(bounds);
snappedMap.fitBounds(bounds);
};
const requestSnapToRoads = () => {
let requests = [];
for (let i = 0; i < points.length; i += 100) {
let params = new URLSearchParams();
params.append('interpolate', 'true');
params.append('key', apiKey)
params.append('path', points
.slice(i, i + 100)
.map(point => `${point.lat},${point.lng}`)
.join('|'));
requests.push(fetch(`https://roads.googleapis.com/v1/snapToRoads?${params}`));
}
return Promise.all(requests).then(responses => Promise.all(responses.map(getJSON)));
};
const applySnapToRoads = (jsons) => {
let lastIndex = -1;
let allSnappedPoints = jsons.map((json, i) => {
for (let snappedPoint of json.snappedPoints) {
if ('originalIndex' in snappedPoint) {
lastIndex = i * 100 + snappedPoint.originalIndex;
}
snappedPoint.cat = points[lastIndex].cat;
}
return json.snappedPoints;
});
return [].concat.apply([], allSnappedPoints);
};
const drawSnappedPoints = (snappedPoints) => {
const getPointAt = (index) => ({
lat: snappedPoints[index].location.latitude,
lng: snappedPoints[index].location.longitude
});
for (let i = 0; i < snappedPoints.length - 1; i++) {
let path = new google.maps.Polyline({
path: [getPointAt(i), getPointAt(i + 1)],
strokeWeight: 5,
strokeColor: colors[snappedPoints[i].cat]
});
path.setMap(snappedMap);
}
};
// chain them!
fetch('output.json')
.then(getJSON)
.then(drawPoints)
.then(requestSnapToRoads)
.then(applySnapToRoads)
.then(drawSnappedPoints)
.catch(console.log);
-
\$\begingroup\$ Sorry for the delayed comment. But without further ado: WOW! This was like really impressive, I really liked the result and been studying it a lot. Couple of questions though, just to understand the rationale and standards, 1) why creating functions with the arrow functions? just to preserve the
this
object? 2) Why one function per step even though it would never (until now) be used again, for readability? 3) I thought one of the plus of Promises was the fact it doesn't need to declare variables out from its scope, so thatlet points;
would never exist or am I really wrong? \$\endgroup\$Antonio Ribeiro– Antonio Ribeiro2016年11月04日 02:07:00 +00:00Commented Nov 4, 2016 at 2:07 -
\$\begingroup\$ 1. Personally, I'd use local
function
declarations as I'm used to ES5 but here arrow functions are used for syntax consistency only 2. When a function does just one simple task it's much easier to understand for anyone (this is explained better in many coding guides) 3. Regardinglet points
, I've used the simplest solution (you use out-of-scope variables anyway). I could have transferred the variable via Promise chain for better encapsulation. \$\endgroup\$woxxom– woxxom2016年11月04日 02:23:03 +00:00Commented Nov 4, 2016 at 2:23
I would use promises only where it's necessary, a couple of the
transformations here can just be put into a regular variable without
problems, the .json()
calls for example.
- Extract duplicate code like the map creation since all the parameters are the same except for the element ID.
- Extract semantically useful code into functions.
- Be consistent with the semicolons.
- Maybe also use functional mapping to iterate over arrays to get of the
manual
for
-loops. - The first nested
return Promise.all(requests).then(...)
should probably be put one layer up so it flows better, that is,... return Promise.all(requests); }).then(function(responses){...})
.
Otherwise yeah, looks good.
Explore related questions
See similar questions with these tags.