Is it bad practice to have a constructor function return a Promise?

I'm trying to create a constructor for a blogging platform and it has many async operations going on inside. These range from grabbing the posts from directories, parsing them, sending them through template engines, etc.

So my question is, would it be unwise to have my constructor function return a promise instead of an object of the function they called new against.

For instance:

var engine = new Engine({path: '/path/to/posts'}).then(function (eng) {
   // allow user to interact with the newly created engine object inside 'then'
   engine.showPostsOnOnePage();
});

Now, the user may also not supply a supplement Promise chain link:

var engine = new Engine({path: '/path/to/posts'});

// ERROR
// engine will not be available as an Engine object here

This could pose a problem as the user may be confused why engine is not available after construction.

The reason to use a Promise in the constructor makes sense. I want the entire blog to be functioning after the construction phase. However, it seems like a smell almost to not have access to the object immediately after calling new.

I have debated using something along the lines of engine.start().then() or engine.init() which would return the Promise instead. But those also seem smelly.

Edit: This is in a Node.js project.

Solution 1:

Yes, it is a bad practise. A constructor should return an instance of its class, nothing else. It would mess up the new operator and inheritance otherwise.

Moreover, a constructor should only create and initialize a new instance. It should set up data structures and all instance-specific properties, but not execute any tasks. It should be a pure function without side effects if possible, with all the benefits that has.

What if I want to execute things from my constructor?

That should go in a method of your class. You want to mutate global state? Then call that procedure explicitly, not as a side effect of generating an object. This call can go right after the instantiation:

var engine = new Engine()
engine.displayPosts();

If that task is asynchronous, you can now easily return a promise for its results from the method, to easily wait until it is finished.
I would however not recommend this pattern when the method (asynchronously) mutates the instance and other methods depend on that, as that would lead to them being required to wait (become async even if they're actually synchronous) and you'd quickly have some internal queue management going on. Do not code instances to exist but be actually unusable.

What if I want to load data into my instance asynchronously?

Ask yourself: Do you actually need the instance without the data? Could you use it somehow?

If the answer to that is No, then you should not create it before you have the data. Make the data ifself a parameter to your constructor, instead of telling the constructor how to fetch the data (or passing a promise for the data).

Then, use a static method to load the data, from which you return a promise. Then chain a call that wraps the data in a new instance on that:

Engine.load({path: '/path/to/posts'}).then(function(posts) {
    new Engine(posts).displayPosts();
});

This allows much greater flexibility in the ways to acquire the data, and simplifies the constructor a lot. Similarly, you might write static factory functions that return promises for Engine instances:

Engine.fromPosts = function(options) {
    return ajax(options.path).then(Engine.parsePosts).then(function(posts) {
        return new Engine(posts, options);
    });
};

…

Engine.fromPosts({path: '/path/to/posts'}).then(function(engine) {
    engine.registerWith(framework).then(function(framePage) {
        engine.showPostsOn(framePage);
    });
});

Solution 2:

I encountered the same problem and came up with this simple solution.

Instead of returning a Promise from the constructor, put it in this.initialization property, like this:

function Engine(path) {
  var engine = this
  engine.initialization = Promise.resolve()
    .then(function () {
      return doSomethingAsync(path)
    })
    .then(function (result) {
      engine.resultOfAsyncOp = result
    })
}

Then, wrap every method in a callback that runs after the initialization, like that:

Engine.prototype.showPostsOnPage = function () {
  return this.initialization.then(function () {
    // actual body of the method
  })
}

How it looks from the API consumer perspective:

engine = new Engine({path: '/path/to/posts'})
engine.showPostsOnPage()

This works because you can register multiple callbacks to a promise and they run either after it resolves or, if it's already resolved, at the time of attaching the callback.

This is how mongoskin works, except it doesn't actually use promises.

Edit: Since I wrote that reply I've fallen in love with ES6/7 syntax so there's another example using that. You can use it today with babel.

class Engine {

  constructor(path) {
    this._initialized = this._initialize()
  }

  async _initialize() {
    // actual async constructor logic
  }

  async showPostsOnPage() {
    await this._initialized
    // actual body of the method
  }

}

Edit: You can use this pattern natively with node 7 and --harmony flag!