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 ?
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 ?
Share Improve this question edited Mar 18, 2014 at 8:35 Willem D'Haeseleer asked Mar 18, 2014 at 8:17 Willem D'HaeseleerWillem D'Haeseleer 20.2k10 gold badges68 silver badges103 bronze badges 7-
This looks uselessly heavy. In case of doubt you could always
cast
the return of the function, I can't see any reason to not provide a directly usable value. – Denys Séguret Commented Mar 18, 2014 at 8:26 - 1 BTW isn't it a primarily opinion based" question ? – Denys Séguret Commented Mar 18, 2014 at 8:27
- It's actually quite a mon anti pattern, I think having this question is useful since it's an anti pattern to do this. – Benjamin Gruenbaum Commented Mar 18, 2014 at 8:28
- 3 The purpose of promises is to make your code simpler and clearer. If using promises makes you add some useless repeated code in many functions, then you're doing them wrong. – Denys Séguret Commented Mar 18, 2014 at 8:32
- 2 Doing this deceives a user of the API into believing that it's implemented asynchronously – RemcoGerlich Commented Mar 18, 2014 at 8:39
2 Answers
Reset to default 10There 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 mon 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);
}
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.