Reputation: 714
I refactored my code have cleaner code without duplication. But I'm wondering if the use of $q.all
is the best option in my scenario...
Code logic:
null
for the scenario without promise).$q.all
waits for the promise and then checks if the return value is something returned by the promise (scenario 1) or null
(scenario 2).Function before refactor
model.updateWish = function(wish) {
var defer = $q.defer();
if (wish.image) {
// Rename temporary image.public_id to wish_id
cloudinaryService.renameImage(wish.image.public_id, wish._id,
function (image) {
// Update wish with renamed image
wish.image = image;
$http.put(URLS.WISH + "/" + wish._id, wish).success(function (wish) {
updateWishlist(wish);
defer.resolve(wish);
console.info("wish updated", wish);
});
});
} else {
$http.put(URLS.WISH + "/" + wish._id, wish).success(function (wish) {
updateWishlist(wish);
defer.resolve(wish);
console.info("wish updated", wish);
});
}
return defer.promise;
}
Code after refactor
model.updateWish = function(wish) {
var defer = $q.defer();
var renamedImagePromise = null;
if (wish.image) {
// Rename temporary image.public_id to wish_id
renamedImagePromise = cloudinaryService.renameImage(wish.image.public_id, wish._id)
.then( function (image) {
// Update wish with renamed image
wish.image = image;
return wish;
});
}
// Wait until renameImagePromise is resolved and send updated wish to server
$q.all([renamedImagePromise]).then(function(wishWithRenamedImage){
if (wishWithRenamedImage[0]) { // $q.all returns an array, wish is in "wishWithRenamedImage[0]"
wish = wishWithRenamedImage[0];
}
$http.put(URLS.WISH + "/" + wish._id, wish).success(function (wish) {
updateWishlist(wish);
defer.resolve(wish);
console.info("wish updated", wish);
});
})
return defer.promise;
}
Both functions work, but I'm wondering if this is the best implementation for my requirements...
Upvotes: 0
Views: 172
Reputation: 48968
Use $q.when
and also avoid the deferred anti-pattern:
model.updateWish = function(wish) {
̶v̶a̶r̶ ̶d̶e̶f̶e̶r̶ ̶=̶ ̶$̶q̶.̶d̶e̶f̶e̶r̶(̶)̶;̶
var renamedImagePromise = null;
if (wish.image) {
// Rename temporary image.public_id to wish_id
renamedImagePromise = cloudinaryService.renameImage(wish.image.public_id, wish._id)
.then( function (image) {
var wishClone = Object.assign({},wish);
// Update wish clone with renamed image
wishClone.image = image;
return wishClone;
});
};
// Wait until renameImagePromise is resolved and send updated wish to server
return $q.when(renamedImagePromise).then(function(wishWithRenamedImage){
var wishToPut = wishWithRenamedImage || wish;
return $http.put(URLS.WISH + "/" + wish._id, wishToPut)
.then(function (resolve) {
var wish = resolve.data;
updateWishlist(wish);
̶d̶e̶f̶e̶r̶.̶r̶e̶s̶o̶l̶v̶e̶(̶w̶i̶s̶h̶)̶;̶
console.info("wish updated", wish);
return wish;
});
});
̶r̶e̶t̶u̶r̶n̶ ̶d̶e̶f̶e̶r̶.̶p̶r̶o̶m̶i̶s̶e̶;̶
};
Out of an abundance of caution, I modified the code to clone the wish
object. When an object reference is passed to a JavaScript function, that function can mutate that object. With functional programming best practices, mutating objects should be avoided.
Upvotes: 2