By Benjamin Gruenbaum

2014-05-22 10:07:00 8 Comments

I was writing code that does something that looks like:

function getStuffDone(param) {           | function getStuffDone(param) {
    var d = Q.defer(); /* or $q.defer */ |     return new Promise(function(resolve, reject) {
    // or = new $.Deferred() etc.        |     // using a promise constructor
    myPromiseFn(param+1)                 |         myPromiseFn(param+1)
    .then(function(val) { /* or .done */ |         .then(function(val) {
        d.resolve(val);                  |             resolve(val);
    }).catch(function(err) { /* .fail */ |         }).catch(function(err) {
        d.reject(err);                   |             reject(err);
    });                                  |         });
    return d.promise; /* or promise() */ |     });
}                                        | }

Someone told me this is called the "deferred antipattern" or the "Promise constructor antipattern" respectively, what's bad about this code and why is this called an antipattern?


@Benjamin Gruenbaum 2014-05-22 10:07:00

The deferred antipattern (now explicit-construction anti-pattern) coined by Esailija is a common anti-pattern people who are new to promises make, I've made it myself when I first used promises. The problem with the above code is that is fails to utilize the fact that promises chain.

Promises can chain with .then and you can return promises directly. Your code in getStuffDone can be rewritten as:

function getStuffDone(param){
    return myPromiseFn(param+1); // much nicer, right?

Promises are all about making asynchronous code more readable and behave like synchronous code without hiding that fact. Promises represent an abstraction over a value of one time operation, they abstract the notion of a statement or expression in a programming language.

You should only use deferred objects when you are converting an API to promises and can't do it automatically, or when you're writing aggregation functions that are easier expressed this way.

Quoting Esailija:

This is the most common anti-pattern. It is easy to fall into this when you don't really understand promises and think of them as glorified event emitters or callback utility. Let's recap: promises are about making asynchronous code retain most of the lost properties of synchronous code such as flat indentation and one exception channel.

@mhelvens 2014-09-25 19:43:31

@BenjaminGruenbaum: I'm confident in my use of deferreds for this, so no need for a new question. I just thought it was a use-case you were missing from your answer. What I'm doing seems more like the opposite of aggregation, doesn't it?

@Benjamin Gruenbaum 2014-09-25 19:47:10

@mhelvens If you're manually splitting a non-callback API into a promise API that fits the "converting a callback API to promises" part. The antipattern is about wrapping a promise in another promise for no good reason, you're not wrapping a promise to begin with so it doesn't apply here.

@mhelvens 2014-09-25 20:05:15

@BenjaminGruenbaum: Ah, I though deferreds themselves were considered an anti-pattern, what with bluebird deprecating them, and you mentioning "converting an API to promises" (which is also a case of not wrapping a promise to begin with).

@Benjamin Gruenbaum 2014-09-25 20:06:44

@mhelvens I guess the excess deferred anti pattern would be more accurate for what it actually does. Bluebird deprecated the .defer() api into the newer (and throw safe) promise constructor, it did not (in no way) deprecate the notion of constructing promises :)

@mhelvens 2014-09-25 20:14:52

@BenjaminGruenbaum: No, of course they didn't deprecate the creation of new promises. But they deprecated the .defer() API, as you say. A bad move, in my opinion, since a 'defer-like' API is more flexible, and clearly required for use-cases such as mine. --- But it's no real problem, of course. Their own documentation explains how express defer() in terms of the promise constructor.

@Maverick 2017-03-07 11:09:36

One question : If I have two scenarios where, if the value exists return the value else make async call and get it. I achieved it by wrapping the whole thing in promise and resolve the promise with data else return the promise from aync call. Is there a better way?

@Aprillion 2017-04-23 17:44:14

@Maverick you can Promise.resolve(42) inside getStuffDone to return existing value, or better yet use Promise.resolve(getStuffDone(666)).then(...).catch(...) instead of just getStuffDone(666).then(...) to make sure you work with a Promise even if getStuffDone is not cooperative enough (like throwing a TypeError) - see You-Dont-Know-JS #trustable-promise

@ghuroo 2017-05-30 01:21:12

Thank you @Roamer-1888 your reference helped me finally figuring out what was my issue. Looks like I was creating nested (unreturned) promises without realising it.

@Bergi 2014-08-29 13:28:03

What's wrong with it?

But the pattern works!

Lucky you. Unfortunately, it probably doesn't, as you likely forgot some edge case. In more than half of the occurrences I've seen, the author has forgotten to take care of the error handler:

return new Promise(function(resolve) {
    getOtherPromise().then(function(result) {

If the other promise is rejected, this will happen unnoticed instead of being propagated to the new promise (where it would get handled) - and the new promise stays forever pending, which can induce leaks.

The same thing happens in the case that your callback code causes an error - e.g. when result doesn't have a property and an exception is thrown. That would go unhandled and leave the new promise unresolved.

In contrast, using .then() does automatically take care of both these scenarios, and rejects the new promise when an error happens:

 return getOtherPromise().then(function(result) {

The deferred antipattern is not only cumbersome, but also error-prone. Using .then() for chaining is much safer.

But I've handled everything!

Really? Good. However, this will be pretty detailed and copious, especially if you use a promise library that supports other features like cancellation or message passing. Or maybe it will in the future, or you want to swap your library against a better one? You won't want to rewrite your code for that.

The libraries' methods (then) do not only natively support all the features, they also might have certain optimisations in place. Using them will likely make your code faster, or at least allow to be optimised by future revisions of the library.

How do I avoid it?

So whenever you find yourself manually creating a Promise or Deferred and already existing promises are involved, check the library API first. The Deferred antipattern is often applied by people who see promises [only] as an observer pattern - but promises are more than callbacks: they are supposed to be composable. Every decent library has lots of easy-to-use functions for the composition of promises in every thinkable manner, taking care of all the low-level stuff you don't want to deal with.

If you have found a need to compose some promises in a new way that is not supported by an existing helper function, writing your own function with unavoidable Deferreds should be your last option. Consider switching to a more featureful library, and/or file a bug against your current library. Its maintainer should be able to derive the composition from existing functions, implement a new helper function for you and/or help to identify the edge cases that need to be handled.

@guest271314 2015-11-23 21:39:08

Are there examples , other than a function including setTimeout , where constructor could be used but not be considered "Promise constructor anitpattern" ?

@Bergi 2015-11-23 22:13:26

@guest271314: Everything asynchronous that doesn't return a promise. Though often enough you get better results with the libraries' dedicated promisification helpers. And make sure to always promisify at the lowest level, so it's not "a function including setTimeout", but "the function setTimeout itself".

@guest271314 2015-11-23 22:19:53

"And make sure to always promisify at the lowest level, so it's not "a function including setTimeout", but "the function setTimeout itself"" Can describe , link to differences , between the two ?

@Bergi 2015-11-23 22:21:29

@guest271314 A function that just includes a call to setTimeout is clearly different from the function setTimeout itself, isn't it?

@guest271314 2015-11-23 22:26:32

Perhaps not aware of clear difference , here. function () {setTimeout(dostuff, duration)} , setTimeout(dostuff, duration) ? Can describe context difference between two , if above two variations are , or are not , accurate representation of " call to setTimeout is clearly different from the function setTimeout" ?

@Bergi 2015-11-23 22:33:56

@guest271314: The lowest level to be promisified is the setTimeout function. Your doStuff should go in a promise callback.

@Oriol 2016-08-29 01:46:20

I can't find any Promise.prototype.done in the spec. Were you referring to the non-standard, or did you mean then?

@Bergi 2016-08-29 01:48:17

@Oriol: Yes, done of Q or jQuery or whatever. Doesn't really matter since its the anti-example anyway, but I'll swap it for the standard one (given that I also use the promise constructor there)

@David Spector 2019-08-27 00:44:18

I think one of the important lessons here, one that has not been clearly stated so far, is that a Promise and its chained 'then' represents one asynchronous operation: the initial operation is in the Promise constructor and the eventual endpoint is in the 'then' function. So if you have a sync operation followed by an asynch operation, put the sync stuff in the Promise. If you have an async operation followed by a sync, put the sync stuff in the 'then'. In the first case, return the original Promise. In the second case, return the Promise/then chain (which is also a Promise).

Related Questions

Sponsored Content

2 Answered Questions

[SOLVED] ECMAScript 6 Chaining Promises

29 Answered Questions

27 Answered Questions

[SOLVED] What is the (function() { } )() construct in JavaScript?

  • 2011-11-22 14:19:11
  • Exitos
  • 290832 View
  • 801 Score
  • 27 Answer
  • Tags:   javascript iife

15 Answered Questions

[SOLVED] How do I tell if an object is a Promise?

17 Answered Questions

7 Answered Questions

[SOLVED] When is .then(success, fail) considered an antipattern for promises?

19 Answered Questions

[SOLVED] How do I convert an existing callback API to promises?

6 Answered Questions

2 Answered Questions

[SOLVED] Why does javascript ES6 Promises continue execution after a resolve?

1 Answered Questions

Sponsored Content