How do I convert this function to not use the Promise constructor antipattern

Question

I asked this question and was told to avoid the Promise constructor anti-pattern. I read through the article linked but I am unsure how to not use the anti pattern. This is the function:

db.getUserCount = function () {
    return new Promise (function (resolve, reject) {
    db.users.count().then(function (result) {
        resolve (result);
    }, function(e) {
        reject (e);
    });
});

I have all my database queries coded like the one above. This is my attempt to not use the anti-pattern but as I don't totally understand it I don't think this way is correct either:

db.getUserCount = function () {
    return new Promise (function (resolve, reject) {
        db.users.count().then(function (result) {
            return (result);
        }).then(function(result) {
            resolve(result);
        }).catch(function(err) {
            reject(err);
        });
    });
};

As I want to be writing good code, if someone could take a minute to show me an example of using the above function by not using the anti-pattern that would be great.

score 5 · Answer 1 · answered May 03 '16 at 12:13

5

Since db.users.count() seems to return a promise already, you can just do this:

db.getUserCount = function () {
    return db.users.count();
});

There's not need to wrap it in an additional promise (using new Promise(...)).

answered May 03 '16 at 12:13

robertklep

198,204
35
394
381

1

Thanks! I was wrapping all my queries in an extra promise and there was no need. That must be the reason I could never reach the reject part in my tests also. – nasoj1100 May 03 '16 at 13:34

score -1 · Accepted Answer · edited May 23 '17 at 12:23

-1

If the db.users.count() function is an asynchronous operation, then what you've written is not an anti pattern. The "anti pattern" comes in when the method is synchronous, and a promise is returned for the sake of using the Promise Pattern.

The key here is from a quote from another Stack Overflow question linked to in the question you reference:

... promises are about making asynchronous code retain most of the lost properties of synchronous code such as flat indentation and one exception channel.

(emphasis, mine)

If the code is not asynchronous, then returning a promise is an anti pattern.

As for returning new Promise(...) being an anti pattern? If that's the case then returning new Anything() is an anti pattern. Using the Promise constructor is fine.

edited May 23 '17 at 12:23

Community

1
1

answered May 03 '16 at 12:13

Greg Burghardt

17,900
9
49
92

1

This is incorrect and misleading. See the other answer for what's the correct way to write the code. – Benjamin Gruenbaum May 03 '16 at 16:12
1

Also, your emphasis is not what the quote means at all. I know because you're quoting me there. Promises give you chaining for free and you only need to use `new Promise` when _converting_ an API to promises, it's pointless in an already promised API. – Benjamin Gruenbaum May 03 '16 at 16:33
@BenjaminGruenbaum: I missed the fact that the code in the question already returned a promise, but my statement still stands: "f the code is not asynchronous, then returning a promise is an anti pattern." It just so happens that the code in the question *is* already asynchronous and returning a promise, making the new promise useless. – Greg Burghardt May 03 '16 at 20:37
Right, so now your statement is correct - but not useful to the OP :P – Benjamin Gruenbaum May 04 '16 at 08:14

How do I convert this function to not use the Promise constructor antipattern

2 Answers2