1
\$\begingroup\$

I've realized JS callback hell and I started looking into Promises libraries. After reading some articles I decided to start with BluebirdJS. I didn't realise how to properly use them yet, because documentation written with API examples, not real usage.

I have two files (NodeJS environment):

#monitor.js
Promise = require("bluebird")
class Monitor
  constructor: (@api) ->

  getServerInfo: ->
    return new Promise((resolve, reject) =>
      @api.query("messages.getLongPollServer", { use_ssl: 1}).then(
        (result) =>
          @key = result.response.key
          @server = result.response.server
          @ts = result.response.ts

          resolve(@)

        reject
      )
    )

  printDetails: ->
    console.log(@key, @server, @ts)

module.exports = Monitor

#app.js
Api = require("./api")
Monitor = require("./monitor")

api = new Api("6de61129da290c3edf9c699ad25638156266e7b6bdbcc00407efb630e5c62f2d4c71974c7bdba52a03dbd")

monitor = new Monitor(api)

monitor.getServerInfo().then((monitor) -> monitor.printDetails())

What I wanted to achieve is something like this:

monitor.getServerInfo().then().printDetails()

because getServerInfo runs asynchronously.

Am I using promises correctly?

\$\endgroup\$
  • \$\begingroup\$ what is Api? Why are you creating new Promise when api.query already returns a promise? \$\endgroup\$ – Esailija Apr 23 '14 at 17:23
2
\$\begingroup\$
  1. Don't use explicit returns in CoffeeScript unless you have to (and you don't here)
  2. CoffeeScript may do automatic comma insertion, but I certainly didn't right away (i.e. add a comma yourself, right before reject - anything to make it easier to read)

Anyway: You can't get this code

monitor.getServerInfo().then().printDetails()

because that would be synchronous, not asynchronous. The interpreter would have to wait in the middle of the chain before it could call printDetails. And it doesn't wait - that's the point of asynchronicity.

And that's why then() always returns a promise object (and nothing else), because it can return that right away without any waiting.

So your current code

monitor.getServerInfo().then((monitor) -> monitor.printDetails())

is the way to do it. Of course, it doesn't make much sense to have a printDetails method in your Monitor, if it doesn't always work. Right now, you can just call

(new Monitor(api)).printDetails()

and get

undefined, undefined, undefined

printed. Or, if you just happened to call it after the API query has run, printDetails will suddenly print actual values. The method is almost random at that point because there's nothing guaranteeing that things happen in the right order.

I'd just skip the Monitor class entirely, since it's not really contributing much - at least not right now. But, given the name "monitor", I imagine the point is to get the server info, so you can then set up a polling system of some kind.

In that case, you could make a factory method that returns its product via a promise:

Promise = require "bluebird"

class Monitor
  constructor: (@server, @key, @ts) ->
    # I don't know if a Monitor instance needs an api object
    # for anything. If it does, add it as an argument to
    # this constructor

  startPolling: ->
    # ... or something

  stopPolling: -> 
    # ... or something

  printDetails: ->
    console.log @server, @key, @ts

# factory function
Monitor.create = (api) ->
  # internal function that actually runs the query
  createMonitor = (resolveCallback, rejectCallback) ->
    success = (data) ->
      {server, key, ts} = data.response # CoffeeScript destructuring
      resolveCallback new Monitor(server, key, ts) # also pass in the api obj, if needed

    api.query("messages.getLongPollServer", use_ssl: 1).then success, rejectCallback

  # return a promise that'll resolve with 
  # a new Monitor instance (or fail,
  # presumably with an error of some sort)
  new Promise createMonitor

module.exports = Monitor

If you already have the server, key and ts values from somewhere, then great; just call new Monitor(server, key, ts) and you're done.

But if you don't, you can do this:

Api     = require("./api")
Monitor = require("./monitor")

api = new Api("6de61....")

success = (monitor) -> monitor.printDetails() # or whatever else you need
failure = (args...) -> console.warn args...

Monitor.create(api).then success, failure

This is basically turning the process inside out: Instead of instantiating a Monitor object, and making it fetch the data it needs, the factory method is doing the fetching, and only creating the instance when it's time to do so.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Bluebird has the concept of Promisification that converts any library into a Promise. Maybe it did not exist at the time the chosen answer was written, but I wanted to add this so people coming into this answer should look into that option before anything else. You can use this without changing your existing library code.

https://github.com/petkaantonov/bluebird/blob/master/API.md#promisification

var Promise = require('bluebird');
var Monitor = Promise.promisifyAll(require('Monitor'));

Then something like:

//using getServerInfoAsync to show that you could have written 
//getServerInfo without returning a Promise, as if it were to be used as a callback
monitor.getServerInfoAsync().then(monitor.printDetailsAsync)
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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