Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it useful to always return a promise

I'm using bluebird to design some nodejs api wrapper around an http service. Many of the functions in this wrapper are asynchronous and so it makes a lot of sense to return promises from these implementation.

My colleague has been working on the project for a few days now and interesting pattern is emerging, he is also returning promises from synchronously implemented functions.

Example:

function parseArray(someArray){
    var result;
    // synchronous implementation
    return Promise.resolve(result);           
}

I can see how this could be useful if later on the implementation needs to be made asynchronous, as you wouldn't have to refactor the call sites. I guess it's also nice that all methods are consistently "async", but I'm not sure how awesome that exactly is.

Is this considered a bad practice, are there any reasons why we shouldn't do this ?

like image 531
Willem D'Haeseleer Avatar asked Mar 18 '14 08:03

Willem D'Haeseleer


2 Answers

There is no point in returning a promise in synchronous methods.

Promises provide an abstraction over concurrency. When there is no concurrency involved such as when providing an array. Returning a promise makes for worse flow control and is considerably slower.

This is also conveying the wrong message. In fact, promisifying things with no reason is quite a common anti pattern.

One case where it is useful is when a method might be asynchronous - for example: fetching something from cache or making a request for it if it's not there:

function getData(id){
     if(cache.has(id) return Promise.cast(cache.get(id));
     return AsyncService.fetch(id).tap(cache.put);
}
like image 191
Benjamin Gruenbaum Avatar answered Oct 21 '22 07:10

Benjamin Gruenbaum


If we can ignore perf it's only bad if the example implementation is used:

function parseArray(someArray) {
    var result;
    // synchronous implementation
    return Promise.resolve(result);           
}

This function is a mess because it can throw synchronously but also returns a promise. A function that returns a promise must never throw - otherwise you have lost most benefits of promises because now you have to use both try-catch and .catch().

The correct way to implement is to "annotate" a function with .method:

var parseArray = Promise.method(function() { 
    var result;
    //Promise.resolve(result) is unnecessary now
    return result;
});

Now the function is guaranteed to never throw synchronously and can be used in a consistent way.

like image 37
Esailija Avatar answered Oct 21 '22 09:10

Esailija