Angular: use $q.all for optional promise?

Question

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:

I have an "optional" promise. In 1 scenario I need to call an external API (= promise), in the other scenario, I don't need that external call (= no promise).
So I created a variable in which I can store the promise (or 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...

Please read about the deferred antipattern: https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns — The Head Rush, May 11 '19 at 14:45

georgeawg · Accepted Answer · 2019-05-12T08:56:48.797

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̶;̶
};

Update

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.

Thanks, refactored my code as you suggested and everything works fine. — bits, May 12 '19 at 08:01

Angular: use $q.all for optional promise?

1 Answers1

Update