1

I'm very new to programming (3 months) and am having trouble with how node handles asynchronous functions (I think).

I have a Merchant class object with a method "addMenu" that makes a GET request for a menu from an external API and then updates the merchant by setting the merchant.data.menu object (which is null by default) to the new menu we just got.

Code in question:

this.addMenu = function(currentMerchant) {
  var id = currentMerchant.id;

  function  getMenu(id) {
    var deferred = Q.defer();
    var url = 'https://api.delivery.com/merchant/'+id+'/menu?client_id=xyz';

    request.get(url, function(error, response, body) {
      if(error) {
        console.log("Something went wrong with menu GET request: Status Code: " + response.statusCode);
        deferred.reject(new Error(error));
      } else if(!error && response.statusCode == 200) {
        menuObj = JSON.parse(body);
        deferred.resolve(menuObj);
      }        
    });

    return deferred.promise;
  };

  this.data.menu  = getMenu(id).then(function(currentMenu) {
    return currentMenu;
  });

  console.log(this.data.menu);
};

When I log (this.data.menu), I get "{ state: 'pending' }." I can do setTimeout and get things to work but doesn't that defeat the whole purpose of promises? I've been stuck on this general problem for days - have been delving into callbacks, delays, promises etc to solve it but am thinking I might be missing something more fundamental in my thinking.

Thank you!

Edit to add:

Well after all that I realized that the real crux of my problem was the inability to access this.data.menu from inside of the callback / promise which lead me to doing all kinds of weird stuff and trying to return them into the this. variable etc.

Just read up on the "var that = this;" trick to gain access to class scope which made all of my callback and promise attempts work fine and make sooo much more sense in my head. And I now know a hell of a lot more about promises that I ever intended as a side benefit. Thanks for the help folks!

greenleaf
  • 21
  • 2
  • 1
    Promises don't magically make asynchronous code synchronous. They can only simplify our treatment of asynchrony. – Bergi Nov 13 '15 at 00:37

2 Answers2

1

The return from the then method is still a promise and so would not be fulfilled. This is a hard paradigm to get your head around at first. (ever played portals? :))

I think you want to do something like this

getMenu(id).then(function(currentMenu) {
    this.data.menu  = currentMenu;
});

The key part of this is you do your assignment inside the function that is called when the promise is fulfilled.

Any code outside of that is not guaranteed to be running after the promise has come back (which is why you are getting that console output)

ScottGuymer
  • 1,885
  • 1
  • 17
  • 22
  • See that makes a lot of sense and is how I originally had it set up. But I don't have access to 'this.data.menu' from within the .then function callback. I guess that's where I'm stuck. And if I try to temporarily assign it to a variable within the addMenu method and then assign that to 'this.data.menu, it only successfully sticks it in there if I add a delay so we're back to square one... Thanks for your help btw! – greenleaf Nov 13 '15 at 18:36
  • It seems like what you are struggling with now is the scope of this within the callback. There are a lot of posts out there that will explain that better than I possibly could. This is a pretty common problem some people advocate never using this and others disagree. Its up to you to make your mind up. Here is an example post http://weblogs.asp.net/dwahlin/working-with-the-javascript-this-keyword – ScottGuymer Nov 16 '15 at 09:18
  • Thanks ScottG, that was exactly my problem... will look into theories on 'this' usage – greenleaf Nov 24 '15 at 17:56
-1

You could also use bluebird to promisify the request and do something like this:

var Promise = require('bluebird');

var getRequest = Promise.promisify(require('http').get);

this.addMenu = function(currentMerchant) {
  var id = currentMerchant.id;

  function  getMenu(id) {
    var deferred = Q.defer();
    var url = 'https://api.delivery.com/merchant/'+id+'/menu?client_id=xyz';

    return getRequest(url);
  }

  this.data.menu  = getMenu(id)
                      .then(function(response) {
                        return JSON.parse(response.body);
                      })
                      .catch(function (err){
                        console.log("Something went wrong with menu GET request: " + err);
                      });

  console.log(this.data.menu);
};
tacotuesday
  • 125
  • 1
  • 5
  • 14
  • Yeah so ditch Q and go with a library that allows you to promisify callbacks. His problem isnt a lack of understanding of promises, its mixing promises with callbacks. – tacotuesday Nov 13 '15 at 04:30
  • No; OP has already correctly set up the request to use a promise; the issue is that they are using the return value incorrectly. Switching libraries won't make a difference. It appears your code suffers from the same confusion. – Evan Davis Nov 13 '15 at 13:08
  • Thanks for suggestion user2640213, messing around with bluebird now. I can see how promisifying the GET request might sidestep my problem but I'm thinking mathletics might be right and I'm missing a bigger piece of the puzzle conceptually – greenleaf Nov 13 '15 at 18:40