patternjavascriptMinor
Flow of Angular Promises
Viewed 0 times
flowangularpromises
Problem
I recently put in a submission for a small coding challenge using Angular. The challenge was to get a token and array of values using a GET request then make another GET request passing the token and sum of values within the url. I'm still getting to grips with promises and want to know if the flow in the service and controller code below can be improved. For me the problematic line is returning the helper function
dataservice.js
```
(function() {
'use strict';
angular
.module('app')
.factory('dataservice', dataservice);
dataservice.$inject = ['$http', '$log'];
function dataservice($http, $log) {
var sum, token, challenge;
return {
getChallenge: getChallenge
};
function getChallenge(url) {
return $http.get(url)
.then(function(data, status, headers, config) {
token = data.data.token;
sum = sumArray(data.data.values);
challenge = {token, sum};
return solveChallenge(url, challenge);
})
.catch(function(data, status, headers, config) {
$log.error("The request failed. Error code: " + status);
});
}
function solveChallenge(url, challenge) {
var sum, token, solutionUrl;
sum = challenge.sum;
token = challenge.token;
solutionUrl = url + '/' + token + '/' + sum;
return $http.get(solutionUrl)
.then(function(data, status, headers, config) {
return data.data.answer;
})
solveChallenge from within the getChallenge function. It works but is my use of promises good or an anti-pattern? Should I be using the success and error methods provided by $http in place of then and catch? Any other feedback on the code is definitely much appreciated.dataservice.js
```
(function() {
'use strict';
angular
.module('app')
.factory('dataservice', dataservice);
dataservice.$inject = ['$http', '$log'];
function dataservice($http, $log) {
var sum, token, challenge;
return {
getChallenge: getChallenge
};
function getChallenge(url) {
return $http.get(url)
.then(function(data, status, headers, config) {
token = data.data.token;
sum = sumArray(data.data.values);
challenge = {token, sum};
return solveChallenge(url, challenge);
})
.catch(function(data, status, headers, config) {
$log.error("The request failed. Error code: " + status);
});
}
function solveChallenge(url, challenge) {
var sum, token, solutionUrl;
sum = challenge.sum;
token = challenge.token;
solutionUrl = url + '/' + token + '/' + sum;
return $http.get(solutionUrl)
.then(function(data, status, headers, config) {
return data.data.answer;
})
Solution
It works but is my use of promises good or an anti-pattern?
Promises work great when using $http.
Should I be using the success and error methods provided by $http in place of then and catch?
According to the documentation:
Returns a promise object with the standard then method and two http specific methods: success and error. The then method takes two arguments a success and an error callback which will be called with a response object. The success and error methods take a single argument - a function that will be called when the request succeeds or fails respectively. The arguments passed into these functions are destructured representation of the response object passed into the then method.
So you should use
but there is nothing wrong with your sumArray function as is.
Promises work great when using $http.
Should I be using the success and error methods provided by $http in place of then and catch?
According to the documentation:
Returns a promise object with the standard then method and two http specific methods: success and error. The then method takes two arguments a success and an error callback which will be called with a response object. The success and error methods take a single argument - a function that will be called when the request succeeds or fails respectively. The arguments passed into these functions are destructured representation of the response object passed into the then method.
So you should use
.then(successHandler, errorHandler) or .success(successHandler) and .error(errorHandler), but .catch(errorHandler) will not work with $http! According to the docs anyway...function solveChallenge(url, challenge) is confusing to me, is this function really solving the challenge or is it checking the solution against a server? Consider renaming to checkSolution(url, challenge)function getChallenge(url) is also confusing, it gets the challenge, solves it, and returns an answer from a server to see if the solution was correct. Should it be renamed to getAndSolveChallenge(url)?function sumArray(items) could be rewritten as: function sumArray(items) {
return items.reduce(function(previousValue, currentItem) {
return previousValue + currentItem;
}, 0);
}but there is nothing wrong with your sumArray function as is.
Code Snippets
function sumArray(items) {
return items.reduce(function(previousValue, currentItem) {
return previousValue + currentItem;
}, 0);
}Context
StackExchange Code Review Q#92126, answer score: 3
Revisions (0)
No revisions yet.