1
\$\begingroup\$

I have demonstrated promise chaining where promise's p1,p2,p3 has to be resolved synchronously.

The code below is using angular.js 1, and here P3 is resolved then P2 and then P1.

<script type="text/javaScript">
            angular.module("myApp",[]);
            angular.module("myApp").controller("myCtrl",myCtrl);
            angular.module("myApp").factory("demoService",demoService);

            demoService.$inject = ["$q","$timeout"];
            myCtrl.$inject = ["$scope","demoService"];

            function myCtrl($scope,demoService){
                var vm = this;
                vm.init = init;

                vm.myList = [];

                function init(){
                    var p1 = demoService.get1000();
                    var p3 = demoService.get3000(); 
                    var p2 = demoService.get2000();

                    p3.then(function(obj){
                        vm.myList.push(obj.name);
                        console.log(obj.name);                      
                        return p2;
                    }).then(function(obj){
                        vm.myList.push(obj.name);
                        console.log(obj.name);
                        return p1;
                    }).then(function(obj){
                        vm.myList.push(obj.name);
                        console.log(obj.name);
                    }); 
                } // end of init
            } // end of myCtrl  

            function demoService($q,$timeout){
                var obj = {};
                obj.get1000 = get1000;
                obj.get2000 = get2000;
                obj.get3000 = get3000;

                return obj;

                function get1000(){
                    var deferred = $q.defer();
                    var INTERVAL = 1000;
                    $timeout(function() {                       
                        deferred.resolve({ "name" : INTERVAL });
                    }, INTERVAL);
                    return deferred.promise;
                }

                function get2000(){
                    var deferred = $q.defer();
                    var INTERVAL = 2000;
                    $timeout(function() {
                        deferred.resolve({ "name" : INTERVAL });
                    }, INTERVAL);
                    return deferred.promise;
                }

                function get3000(){
                    var deferred = $q.defer();
                    var INTERVAL = 3000;
                    $timeout(function() {
                        deferred.resolve({ "name" : INTERVAL });
                    }, INTERVAL);
                    return deferred.promise;
                }

            } // end of demoService
</script>

So, my question is, Is there any better way of doing the above operation ? Also please review my code.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

First of all, unrelated to your query, you can just inline the dependencies and the body of your factory and controller. It's bulky, but less visual noise. Also allows you to write them in separate files easily.

angular.module("myApp").controller("myCtrl",['$scope', 'demoService'], function($scope, demoService){
  // body
}])

// A more concise version. You'll need ng-annotate if you minify this
angular.module('myApp').controller('myCtrl', function($scope, demoService){
  // body
})

Also, semi-colons are optional in JS. Suggesting your remove them, they're just visual noise. Also convert double-quotes to single-quotes. They both mean the same thing, but the latter is is less visually annoying.

As for your query, first issue with your code is it's not doing what you think it's doing. The moment you call get1000, get2000, and get3000, your timers are already running. The only thing synchronous that's happening in that chain is when they get pushed into the array. In this case, your code can easily just use Promise.all().

function init(){
  var p1 = demoService.get1000()
  var p2 = demoService.get2000()
  var p3 = demoService.get3000()

  Promise.all([p3, p2, p1]).then(results => {
    vm.myList.push(...results.map(obj => obj.name))
  })
} 

If you really want to make them synchronous (i.e. call get3000 then push, call get2000 then push, call get1000 then push), you will have to do the following:

function init(){
  demoService.get3000().then(obj => {
    vm.myList.push(obj.name)

    // Only call get2000 if get3000 resolves.
    return demoService.get2000()
  }).then(obj => {
    vm.myList.push(obj.name)

    // Only call get1000 if get2000 resolves.
    return demoService.get1000()
  }).then(obj => {
    vm.myList.push(obj.name)

    // get1000 completed
  })
}

If you're stuck with an environment that does not support async/await, the above syntax (which you're already doing) is pretty much all you've got. You can use arrow functions to make it less noisy. If you can do async/await, then it could be rewritten like so:

async function init(){
  const p1 = await demoService.get1000()
  vm.myList.push(p1)
  const p3 = await demoService.get3000()
  vm.myList.push(p2)
  const p2 = await demoService.get2000()
  vm.myList.push(p3)
}
\$\endgroup\$
8
  • \$\begingroup\$ -1 Automatic semicolon insertion (ASI) is a major flaw in ECMAScript There is talk that ASI will be removed from future ECMAScript in favor of explicit semicolon use. The arguments for the change are strong. Advising that semicolons are just noise without warning of when they are needed, the many complications that can result, or at least a link to the pros and cons, is very poor advice. Also apostrophes (AKA single quotes) can be easily confused with the back tick used to delimit template strings. For this reason you should use quotes to delimit strings, not apostrophes. \$\endgroup\$
    – Blindman67
    Feb 1, 2018 at 15:42
  • \$\begingroup\$ @Blindman67 Sources? \$\endgroup\$
    – Joseph
    Feb 1, 2018 at 15:46
  • \$\begingroup\$ theregister.co.uk/2018/01/12/… \$\endgroup\$
    – Blindman67
    Feb 1, 2018 at 15:53
  • \$\begingroup\$ @Blindman67 There has been talk to add comprehensions in ECMAScript for ages, but they were removed. So going by bleeding edge isn't always a good idea. Also ECMAScript is meant to be backward compatible, so I can't imagine they'd remove ASI without having a special flag - say using "strict";. \$\endgroup\$
    – Peilonrayz
    Feb 1, 2018 at 15:54
  • 1
    \$\begingroup\$ I am not arguing any point, I am trying to explain why I -1 your post. If you give advice it should be sound, include warning where applicable, and not be based on a vagary such as "less visually annoying" \$\endgroup\$
    – Blindman67
    Feb 1, 2018 at 16:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.