if/else with async code in one case

Question

I don't have much experience in async code and I'm stuck with a problem, here is my code :

if (!req.params.user_name) {
    req.params.user = req.me;
} else {
    User.findOne({username: req.params.user_name}, '_id', (error, user) => {
        if (error) {
            return next(error);
        }
        if (!user) {
            let error = new Error('User not found');
            return next(error);
        }
        req.params.user = user;
    });
}
Account.findOne({name: req.params.account_name, created_by: req.params.user})
.populate(['currency', 'created_by'])
.exec((err, account) => {
    if (err) {
        return next(err);
    }
    return res.send(account);
});

As you can see the problem is that in one case I just have a simple procedural action to do, in the other I have to query the database which is async, then I have to execute the code below. I can't just simply put the Account query in the callback of the User query because I don't need to execute User query all the time. I've tried to find an answer here but the only results I've found are about executing one async task or another (ex: Working with promises inside an if/else). Following the recommandations on this post I've thought about wrapping the code inside the if block in an anonymous function and do something like:

let get_user;
if (!req.params.user_name) {
    let get_user = () => {req.params.user = req.me};
} else {
    let get_user = User.findOne({username: req.params.user_name}, '_id', (error, user) => {
        if (error) {
            return next(error);
        }
        if (!user) {
            let error = new Error('User not found');
            return next(error);
        }
        req.params.user = user;
    });
}
get_user().then(() => {
    Account.findOne({name: req.params.account_name, created_by: req.params.user})
        .populate(['currency', 'created_by'])
        .exec((err, account) => {
            if (err) {
                return next(err);
            }
            return res.send(account);
        });
});

But I find it weird and I guess I would need to return a Promise from the anonymous function in the if block.

So what would be an elegant way of solving this ? I'm learning node.js on my free time and any general suggestions would be greatly appreciated.

Thanks for your time.

score 1 · Accepted Answer · answered Aug 09 '18 at 10:30

1

Callbacks shouldn't be used with Mongoose; it has been supporting promises for a long time.

This

let get_user = () => {req.params.user = req.me};

won't work because get_user is expected to return a promise.

It's usually done like:

let userPromise;
if (!req.params.user_name) {
    userPromise = Promise.resolve(req.me);
} else {
    userPromise = User.findOne(...);
}
userPromise().then((user => {
    req.params.user = user;

    return Account.findOne(...);
});

Notice that req.params.user is common code for both conditions. This is conveniently done with async..await, which is syntactic sugar for promises:

try {
    let user;
    if (!req.params.user_name) {
        user = req.me;
    } else {
        user = await User.findOne({username: req.params.user_name}, '_id');
    }

    req.params.user = user;

    const account = await Account.findOne({name: req.params.account_name, created_by: req.params.user})
        .populate(['currency', 'created_by'])
        .exec();

    res.send(account);
} catch (err) {
    next(err);
}

This is supposed to be a body of async middleware function, which wasn't shown in original code.

As explained in this answer, Express doesn't support promises itself, all async middlewares and route handlers should be wrapped with try..catch for error handling.

answered Aug 09 '18 at 10:30

Estus Flask

206,104
70
425
565

Ok, yesterday I did something like that to make it work : if (!req.params.user_name) { get_user = () => { return new Promise((resolve) => { Same for the else block and then would do "get_user().then(() => {" with the Account request inside. So by reading your code I see that I don't need to instantiate a Promise in the if block, just call it statically and that I don't need to wrap it in an anonymous function, same for the else block in which I'll resolve the result of User.findOne. Do you confirm? It is a bit unsettling for me to put sync code into a Promise but it makes more sense now. – user3803848 Aug 09 '18 at 13:34
As for the async..await example does it mean that the User.findOne will return the result automatically if I don't put a callback and automatically throw the error if there's one? That would be great and is more intuitive since it looks like sync code. Thank you for those explanations, it really cleared up those things in my mind. – user3803848 Aug 09 '18 at 13:38
Yes, you don't need anonymous function at all here. You need `Promise.resolve` in case you use raw promises because you need something to chain with `then`, that's why there's `userPromise` in the first snippet. User.findOne returns *a promise of the result*. `await` is syntactic sugar for .`then(result => /* handle result */, err => /* throw an error */)`. Two snippets are functionally equivalent, second snippet looks more straightforward. Since you're using Node which supports async/await, I'd suggest to stick to async/await. – Estus Flask Aug 09 '18 at 13:46
Alright, I'll give a try at async..await, that'll allow me to clean a lot of nested callbacks I have everywhere. Thank you for your time. – user3803848 Aug 09 '18 at 13:56

if/else with async code in one case

1 Answers1