2

I have async function that returns true or false. It looks like following one:

class User {
  async canManageGroup(group) {
    if (typeof group === 'number') {
      // group - id
      group = await getGroupById(group)
    } // else group is already loaded from DB

    return this.id === group.manager.id
  }
}

If group parameter is ID of group then function will make async call to DB, so function canManageGroup will execute asynchronously. But if group parameter is group model then function will only call return this.id === group.manager.id or it will execute syncronously. Is it good practice to write code in this way? Or I should transform synchronous code to asynchronous?

function makeAsync(cb) {
  return new Promise(resolve => setImmediate(() => resolve(cb())))
}

class User {
  async canManageGroup(group) {
    if (typeof group === 'number') {
      // group - id
      group = await getGroupById(group)
    } // else group is already loaded from DB

    return await makeAsync(() => this.id === group.manager.id)
  }
}
Improve this question
2
  • no, don't do the second one ... that's redundant in an async function Commented Apr 18, 2018 at 13:03
  • Possible resolution: have canManageGroup always expect a group (never a group ID); if the caller only has a group ID, then they can call canManageGroup(await getGroupById(groupId)) Commented Feb 20, 2022 at 18:59

3 Answers 3

Reset to default
1

You can use the first example without problems.

When you use async, your function will return a Promise. If your code is sync, the status of the returned promise will be resolved and it is safe to run any promise-related code on it (then, catch, etc).

For example:

async function truePromise() {
  return true;
}

truePromise().then(function(value){
  console.log("Promise value:", value);
});

Just works :)

Should you do it?

Yes. It is okay and works okay thanks to the async keyword.

What you MUST NOT do is the following:

function dontDoIt(doSync) {
  if (doSync) return false;
  return Promise.resolve(false)
}

Why? Because:

  • if doSync is truhtly, it will return false (i.e a boolean)
  • if doSync is falsy, it will return a Promise that will resolve to false.

That is a HUGE difference.

Why?

Your function sometimes returns a promise, and other times return a boolean. It is inconsistent.

  • doSync(true) is a boolean, you can't use await nor .then.
  • doSync(false) is a Promise, you can use await and then.
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

I know that this code will work. But is it good to write sync code in async function? According to this post stackoverflow.com/a/22286961/9363390 I shouldn't write sync code in async function
Updated my answer. TLDR; if you use async keyword, it is okay. When you don't, you could have inconsistency problems. And btw, the problem of consistence pointed by the author of your linked answer is not a problem when you use async keyword.
0

Based on your comment to one of the answers you are concerned because of the answer to the question How do you create custom asynchronous functions in node.js?

The problem is, this function is inconsistent: sometimes it is asynchronous, sometimes it isn't. Suppose you have a consumer like this:

In js enviroments like nodejs is common practice that the callback that could be executed async, should always be called async, no matter if the actual code was really async.

So you are wondering if the following code will break that common practie.

async function doSomething() {
  return true
}

doSomething()
.then(res => {
   console.log(res)
})

This is not the case, because here you deal with Promises and a Promise is allowed to be resolve immediatly, the code above is basicly the same as if you would write:

Promise.resolve(true)
.then(res => {
   console.log(res)
})

The async behaviour is ensured in the chaining part (then/catch), the callback you pass to the then/catch will be called async:

async function doSomething() {
  return true
}

console.log('before');

doSomething()
  .then(res => {
    console.log('with in callback')
  })

console.log('after');

As you can see the order of the logs is:

before
after
with in callback

So this is in the same order you would expect from an regular async callback function.

Improve this answer

Comments

0

The first one is correct. There is no need to force anything to async if there doesn't need to be. Context switching isn't free, so if it doesn't have to wait for anything to complete, don't try and make it.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.