This is the first time I'm trying to use promises in JavaScript so I'm not sure if I have done it correctly, but it seems to work. The problem is that I have ended up with a "nested when
" and it doesn't look that great. There probably is a nicer solution out there. I would really appreciate some feedback on how I can improve my code.
I am using the navigator.geolocation
to get the position and then I'm using two different APIs to get the current weather and the address of the location.
What I would like it to look like would be something like:
$.when(getPosition())
.then(getWeather(position),getLocationName(position))
.done(displayWeather);
But it doesn't seem to work with two calls in then()
like you can have in when()
.
This is my current solution:
var displayWeather = function(weatherData, locationData) {
$('#userLocation').text(locationData.results[3].formatted_address);
$('#weatherDescription').text(weatherData.currently.temperature);
$('#temperature').text(weatherData.currently.summary);
}
var getPosition = function() {
var deferred = $.Deferred();
navigator.geolocation.getCurrentPosition(deferred.resolve, deferred.reject);
return deferred.promise();
};
var getWeather = function(position) {
return $.getJSON('https://api.forecast.io/forecast/{APIKEY}/' + position.coords.latitude + ',' + position.coords.longitude + '?callback=?')
.then(function(data) {
return data;
});
}
var getLocationName = function(position) {
return $.getJSON('https://maps.googleapis.com/maps/api/geocode/json?latlng=' + position.coords.latitude + ',' + position.coords.longitude + '&key={APIKEY}')
.then(function(data) {
return data;
});
}
$(document).ready(function() {
$.when(getPosition())
.done(function(position) {
$.when(getWeather(position), getLocationName(position)).done(displayWeather);
});
});
2 Answers 2
$.when(getPosition())
$.when
is only used when listening to multiple promises. Since getPosition
is just one and it already returns a promise, $.when
isn't required and you can simply chain then
to getPosition()
.
var getWeather = function(position) {
return $.getJSON('https://api.forecast.io/forecast/{APIKEY}/' + position.coords.latitude + ',' + position.coords.longitude + '?callback=?')
.then(function(data) {
return data;
});
}
var getLocationName = function(position) {
return $.getJSON('https://maps.googleapis.com/maps/api/geocode/json?latlng=' + position.coords.latitude + ',' + position.coords.longitude + '&key={APIKEY}')
.then(function(data) {
return data;
});
}
Attaching then
which just returns the resolved data data
is unnecessary. $.getJSON
already returns a promise-like object that resolves to data
. Any then
attached to it will get data
in the same way this then
receives it.
Additionally, suggesting you use then
instead of the jQuery-specific done
. then
is standard, and allows you to easily move over to the standard Promises.
I usually recommend pushing off the "flow logic" of promises to the caller instead of spreading it across different functions. This allows you to easily write functions as promise-returning operations while keeping all the "flow logic" in one place.
On other stuff, suggesting you move the url into a variable. This keeps your AJAX operation short and free from the string concatenation madness. A;so recommending using named function instead of function expressions. They're easier to debug as their names appear in the stack trace.
Your code could be simplified into:
function getPosition () {
return $.Deferred(function(deferred){
navigator.geolocation.getCurrentPosition(deferred.resolve, deferred.reject);
}).promise();
};
function getWeather (position) {
var url = 'https://api.forecast.io/forecast/{APIKEY}/' + position.coords.latitude + ',' + position.coords.longitude + '?callback=?'
return $.getJSON(url);
}
function getLocationName (position) {
var url = 'https://maps.googleapis.com/maps/api/geocode/json?latlng=' + position.coords.latitude + ',' + position.coords.longitude + '&key={APIKEY}';
return $.getJSON(url);
}
$(document).ready(function() {
getPosition().then(function(position){
return $.when(getWeather(position), getLocationName(position));
}).then(function(weatherData, locationData){
$('#userLocation').text(locationData.results[3].formatted_address);
$('#weatherDescription').text(weatherData.currently.temperature);
$('#temperature').text(weatherData.currently.summary);
});
});
If you can write using ES6, the code becomes simpler and you can drop the jQuery dependency for the data gathering functions. ES6 now has native promises as well as template strings, and modern browsers have fetch
. This reduces jQuery footprint to only the DOM-interacting operations.
function getPosition() {
return new Promise((resolve, reject) => navigator.geolocation.getCurrentPosition(resolve, reject));
};
function getWeather(position) {
const url = 'https://api.forecast.io/forecast/${APIKEY}/${position.coords.latitude},${position.coords.longitude}?callback=?';
return fetch(url).then(response => response.json());
}
function getLocationName(position) {
const url = 'https://maps.googleapis.com/maps/api/geocode/json?latlng=${position.coords.latitude},${position.coords.longitude}&key=${APIKEY}';
return fetch(url).then(response => response.json());
}
$(document).ready(function() {
getPosition()
.then(position => Promise.all(getWeather(position), getLocationName(position)))
.then(function(data){
const weatherData = data[0];
const locationData = data[1];
$('#userLocation').text(locationData.results[3].formatted_address);
$('#weatherDescription').text(weatherData.currently.temperature);
$('#temperature').text(weatherData.currently.summary);
});
});
-
\$\begingroup\$ Thank you so much for the answer! One thing that didn't work for me was to remove the
then
after the$.getJSON
like you suggested. TheweatherData
andlocationData
is then an array instead of a json object. I will definitely try to write it using ES6 instead though. Looks a lot nicer! Thanks again! \$\endgroup\$Veronika– Veronika2016年05月28日 19:12:44 +00:00Commented May 28, 2016 at 19:12
You could use native promises... It took me some experimenting to get used to the new javascript features like destructuring, template strings, .reduce
, and promises, but I like how the code looks...
You can return a Promise
from getPosition:
var getPosition = () => new Promise((resolve, reject) =>
navigator.geolocation.getCurrentPosition(resolve, reject))
$.getJSON
result can be treated like a promise. So getWeather
can take coords
and destructure into latitude
and longitude
right in the argument list, then using template strings you can just return $.getJSON()
var getWeather = ({ latitude, longitude }) =>
$.getJSON(`https://api.forecast.io/forecast/${FORECAST_API_KEY}/${latitude},${longitude}?callback=?`);
You can call getPosition()
and call Promise.all()
with promises to get the weather and location data at the same time. You can modify the value you return from .then()
or return another promise from it. So what I do here is convert the result of getWeather()
into an object with a weather
property with the result, same for location
, then use .reduce()
on Promise.all
waiting on those two promises, then Object.assign()
to merge all those results into one object like { position: {}, weather: {}, location: {} }
$(() => {
getPosition()
.then(position =>
Promise.all([
getWeather(position.coords).then(weather => ({ weather })),
getLocation(position.coords).then(location => ({ location }))
]).then(all => all.reduce((acc, val) => Object.assign(acc, val), { position }))
)
.then(data => {
console.log('data:', data);
displayWeather(data.weather, data.location);
})
.catch(err => alert('ERROR!\r\n' + JSON.stringify(err)))
})
So Promise.all()
ends up with an array that looks like this (...
is the data from the services):
[{weather:{...}, {location:{...}}]
Reduce starts with the accumulator (acc
) as {postion:{...}}
and operates on each element in that array calling Object.assign with the accumulator and the array value, returning the result with merged properties.
The Promise.all()
could be changed to use the array indices with the raw results and it looks cleaner, but I like to avoid accidentally using the wrong index for results. Also you can easily just add another ajax call without changing the then
...
Promise.all([
getWeather(position.coords),
getLocation(position.coords)
]).then(all => ({
weather: all[0],
location: all[1],
position
}))
I inserted the code as a snippet, but I can't seem to enable location running it. It works on jsfiddle though if you set the api keys...
let FORECAST_API_KEY = 'GET YOUR OWN';
// https://darksky.net/dev/account
let GOOGLE_API_KEY = 'GET YOUR OWN';
// https://developers.google.com/maps/documentation/geocoding/get-api-key
var displayWeather = function(weatherData, locationData) {
$('#userLocation').text(locationData.results[3].formatted_address);
$('#weatherDescription').text(weatherData.currently.summary);
$('#temperature').text(weatherData.currently.temperature);
}
var getPosition = () => new Promise((resolve, reject) => {
navigator.geolocation.getCurrentPosition(resolve, reject);
});
var getWeather = ({ latitude,longitude }) => {
return $.getJSON(`https://api.forecast.io/forecast/${FORECAST_API_KEY}/${latitude},${longitude}?callback=?`);
}
var getLocation = ({latitude, longitude }) => {
return $.getJSON(`https://maps.googleapis.com/maps/api/geocode/json?latlng=${latitude},${longitude}&key=${GOOGLE_API_KEY}`)
}
$(() => {
getPosition()
.then(position =>
Promise.all([
getWeather(position.coords).then(weather => ({ weather })),
getLocation(position.coords).then(location => ({ location }))
]).then(all => all.reduce((acc, val) => Object.assign(acc, val), { position }))
)
.then(data => {
console.log('data:', data);
displayWeather(data.weather, data.location);
})
.catch(err => alert('ERROR!\r\n' + JSON.stringify(err)))
})
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<p>
Replace <a href="https://darksky.net/dev/account">forecast.io</a> and
<a href="https://developers.google.com/maps/documentation/geocoding/get-api-key">google</a> api keys with your own!
</p>
<p><span id="userLocation">Loading...</span></p>
<p><span id="weatherDescription">Loading...</span></p>
<p><span id="temperature">Loading...</span></p>
-
\$\begingroup\$ This is a very clean and modern way of doing things. Also, one may look at
forkJoin()
as an option. \$\endgroup\$Igor Soloydenko– Igor Soloydenko2017年05月09日 03:32:28 +00:00Commented May 9, 2017 at 3:32
Explore related questions
See similar questions with these tags.