This may sound like a really simply/stupid question but I need to ask it as I haven't came across this scenario before... okay I have a service in my angularJS app. this service currently contains 4 methods that all perform 80% the same functionality/code and I wish to make this more efficient. Here is what my service looks like (with a lot of code removed):
.factory('townDataService', function ($http) {
var townList = {};
townList.getTownList = function () {
return $http({method: 'GET', url: '/api/country/cities'})
.then(function (response) {
// HERE WE FORMAT THE response as desired... that creates a returnArray
var returnArray = [];
// loop through the countries
var JsonData = response.data;
for (key in JsonData['countries']) {
// formatting code...
}
// end of repeated CODE
return returnArray; // this is array, we don't do any formatting here
});
};
townList.getCurrentTown = function (place) {
return $http({method: 'GET', url: '/api/country/cities'})
.then(function (response) {
// HERE WE FORMAT THE response as desired... that creates a returnArray
var returnArray = [];
// loop through the countries
var JsonData = response.data;
for (key in JsonData['countries']) {
// formatting code...
}
// end of repeated code
// now the format further / work with the returnArray...
for (var i = 0; i < returnArray.length; i++) {
// do stuff
}
return currentTown; // this is a string
});
};
townList.getCurrentCountry = function (place) {
return $http({method: 'GET', url: '/api/country/cities'})
.then(function (response) {
// HERE WE FORMAT THE response as desired... that creates a returnArray
var returnArray = [];
// loop through the countries
var JsonData = response.data;
for (key in JsonData['countries']) {
// formatting code...
}
// end of repeated code
// now the format further / work with the returnArray...
for (var i = 0; i < returnArray.length; i++) {
// do stuff
}
return currentCountry; // this is a string
});
};
return townList;
}
)
;
Now I repeat the same $http 'GET' in each method and the same formatting code (which is a lot of nested loops) before returning a object array or a string. This is far from efficent! What is the best way to put this functionality into it's own function so we only call the GET url once but still return a promise with each method? Should I set the results of the $http({method: 'GET', url: '/api/country/cities'}) as a var and inject / pass it into each method before formatting the data if necessary? Should I use some sort of $cacheFactory?
Sorry if this is a dumb question and if I haven't explained myself well I shall rephrase the questions.
Thanks in advance.
It is just as you say; this code can (and should) be refactored in many ways. One example:
Let us factor the HTTP stuff into a separate service, that will also take care of caching. (Another idea for this would be to have a service for the HTTP/remote calls and another - maybe a general use decorator - to handle caching. LEt us not go into so much detail for now.) And let us put the formatting code in another method:
The remote call service:
.service('townHttpService', function($http, $q) {
var cache;
function getCities() {
var d = $q.defer();
if( cache ) {
d.resolve(cache);
}
else {
$http({method: 'GET', url: '/api/country/cities'}).then(
function success(response) {
cache = response.data;
d.resolve(cache);
},
function failure(reason) {
d.reject(reason);
}
});
}
return d.promise;
}
function clearCache() {
cache = null;
}
return {
getCities: getCities,
clearCache: clearCache
};
})
The formatter:
.service('townFormatter', function() {
return function townFormatter(jsonData) {
// HERE WE FORMAT THE response as desired... that creates a returnArray
var returnArray = [], key;
// loop through the countries
for (key in jsonData['countries']) {
// formatting code...
}
// end of repeated CODE
return returnArray; // this is array, we don't do any formatting here
};
})
Your townDataService, written in terms of the above:
.factory('townDataService', function (townHttpService, townFormatter) {
var townList = {};
townList.getTownList = function () {
return townHttpService.getCities().then(townFormatter);
};
townList.getCurrentTown = function (place) {
return townHttpService.getCities().then(townFormatter).then(function(cityList) {
var currentTown;
for (var i = 0; i < cityList.length; i++) {
// do stuff
}
return currentTown; // this is a string
});
};
townList.getCurrentCountry = function (place) {
return townHttpService.getCities().then(townFormatter).then(function(cityList) {
var currentCountry;
for (var i = 0; i < cityList.length; i++) {
// do stuff
}
return currentCountry; // this is a string
});
return townList;
})