4

I would like to utilize the Promises in my class methods. In Promise antipatterns I read that creating a new promise for each new function is considered to be bad.

However, I don't want to return un-related promises in my project, so I thought of doing something like this:

class MyClass {

  async getToken() {
    return new Promise(
      (resolve, reject) => {
        // . . .
        const token_str = '<response_from_http_request>';
        resolve(token_str);
      }
    )
  }

  async doSomething(token) {
    return new Promise(
      (resolve, reject) => {
        const result = // . . .
        resolve(result);
      }
    )
  }

  async doAnotherSomething(token) {
    return new Promise(
      (resolve, reject) => {
        const result = // . . .
        resolve(result);
      }
    )
  }

}

Then I would use it like this:

let instance = new MyClass();

(async () => {
    const token = await instance.getToken();

    const result1 = await instance.doSomething(token);
    console.log(result1);

    const result2 = await instance.doAnotherSomething(token);
    console.log(result2);

})();

Does this seem like a valid way to do this, or is this an antipattern too? And if so, how can I avoid writing code like this?


EDIT: What if I need to make several sequential http calls, perform some actions on the results and then return a Promise based on it?

The way I understand, if I don't make a new Promise, I have to return the one made by the got.js library, which includes the http response data.

Instead, I want to return a Promise which contains the result of my class method.

Example:
  async getCityWeather( city_name ) {
    return new Promise(
      (resolve, reject) => {

        // get the city id based on its name
        const city_id = await got(`https://my-api/getCityIdByName/${city_name}`);

        // now get the weather info for the city with id `cityid`
        const weather_info = await got(`https://my-api/weatherById/${city_id}`);

        // make an object to return
        const temperature = {
          weather_info.temp_min,
          weather_info.temp_max,
        }

        resolve(temperature);

        // ... all error handling are omitted

      }
    )
  }

I don't want to return a Promise that contains got.js return values, I want to return my values based on the http request calls.

ConfusedGuy
  • 121
  • 9
  • 4
    What exactly are all those `// . . .`? Unless you are doing anything asynchronous in there, you should not use promises at all. – Bergi Feb 06 '19 at 13:47
  • 3
    Sidenote: Your `result1` and `result2` only depend on the token. You then can do: `const [result1, result2] = await Promise.all([instance.doSomething(token), instance.doAnotherSomething(token)]);` to not cause blocking. – k0pernikus Feb 06 '19 at 14:00
  • 2
    @k0pernikus Neither way causes blocking in the sense that it blocks the thread. They are wasteful by virtue of the fact that one needs to wait of the other though. – nicholaswmin Feb 06 '19 at 14:02
  • @Bergi I forgot to mention, in all `// . . .` marked lines I'm doing async [got.js](https://stackoverflow.com/q/54486324/11002836) calls. – ConfusedGuy Feb 06 '19 at 15:00
  • @k0pernikus Very good observation! But doesn't this fail to populate a `result*` variable if the other one fails? At least that's what I understand from `Promise.all()`. In any case, using `await` where the calls are independent is wasteful as @Nik said. – ConfusedGuy Feb 06 '19 at 15:03
  • @ConfusedGuy Then yes, you definitely should avoid the `new Promise` constructor antipattern – Bergi Feb 06 '19 at 15:05
  • @Bergi I added a note to the end of my question explaining my understanding, please correct me if I'm mistaken on how the `Promise` value works when chaining them. I want to pass my _own_ results instead of the one made by `got.js` library. – ConfusedGuy Feb 06 '19 at 15:17
  • Use `then` or `await` for that. You don't need any `new Promise` for this. See the `getFooBarBaz` method in the answer below – Bergi Feb 06 '19 at 15:19

1 Answers1

6

async functions always return a Promise.

A function/method will return a Promise under the following circumstances:

  • You explicitly created and returned a Promise from it's body.
  • You returned a Promise that exists outside the method.
  • You marked it as async.

Since you can await a Promise and instance.doSomething is already an async-marked method, you can await it without needing to explicitly return a Promise.

Simply return it's result like you would in a regular synchronous method.

I don't want to return un-related promises in my project...

Unless you're actually doing something asynchronous in your method (accessing the file system, database calls, timers etc...), you don't need to wrap it in a Promise, nor await it when you need a result.

The most usual case where you actually need to wrap something in a Promise is if you have an asynchronous function that works using callbacks but you want to use it as a Promise.

// plain old callback-style asynchronous functions:
const getFooViaCallback = callback => {
  setTimeout(() => {
    callback('foo')
  }, 150)
}

const getBarViaCallback = callback => {
  setTimeout(() => {
    callback('bar')
  }, 150)
}

class Foo {
  constructor() {}
  
  getFooViaPromise() {
    // wrap callback-style code in a Promise
    // so we can await it.
    return new Promise(resolve => {
      getFooViaCallback(result => {
        resolve(result)
      })
    })
  }

  getBarViaPromise() {
    // wrap callback-style code in a Promise
    // so we can await it.
    return new Promise(resolve => {
      getBarViaCallback(result => {
        resolve(result)
      })
    })
  }
  
  getBaz() {
    // no reason to wrap this in a Promise,
    // since it's a synchronous method.
    return 'baz'
  }
  
  async getFooBarBaz() {
    const foo = await this.getFooViaPromise()
    const bar = await this.getBarViaPromise()
    const baz = this.getBaz()

    return foo + ',' + bar + ',' + baz
  }
}

;(async() => {
  const foo = new Foo()
  
  const result = await foo.getFooBarBaz()
  console.log('foo.getFooBarBaz() result: ', result)
})()

I've ommited error handling in the above snippet for brevity but you should use throw in async-marked methods to raise errors. It's the equivalent of calling .reject() within a Promise.

nicholaswmin
  • 21,686
  • 15
  • 91
  • 167
  • 1. Can you explain your first point? How can I use `resolve()` and `reject()` without using `new Promise()` in my code? – ConfusedGuy Feb 06 '19 at 13:21
  • It's the other way round, isn't it? If you're returning a promise you don't need to make the method async too. – Andy Feb 06 '19 at 13:22
  • 2. I'm using [got.js](https://stackoverflow.com/q/54486324/11002836) which requires the `async` context to work. – ConfusedGuy Feb 06 '19 at 13:22
  • @ConfusedGuy functions marked with `async` can use `return` much like a synchronous function. That functions much like calling `resolve` within a `Promise`. Gimme 5' and I'll also post a code snippet that explains this. – nicholaswmin Feb 06 '19 at 13:23
  • @Andy I didn't test this, so sorry for asking _BUT_ does this mean that if I return a Promise, I can use `await` in the method context? – ConfusedGuy Feb 06 '19 at 13:23
  • @NikKyriakides Oh, I understand. I presume `throw Error("Something")` should be used instead of `reject( new Error("Something") )`, then? – ConfusedGuy Feb 06 '19 at 13:25
  • Yes. You can return a promise and use `async`. Something [like this](https://jsfiddle.net/8g3La2so/1/) perhaps. – Andy Feb 06 '19 at 13:28
  • 1
    @ConfusedGuy That's entirely correct, `async` functions must use `throw` to raise errors. – nicholaswmin Feb 06 '19 at 13:32
  • 1
    @ConfusedGuy No, you don't need an "async context" to make got.js work. It returns promises, that's all. You only need an `async` function context when you want to `await` such a promise. – Bergi Feb 06 '19 at 13:49
  • 1
    @Andy It works both ways. If you explicitly return a promise, you don't need to make the function `async`, but conversely, if you make the function `async` you don't have to return a promise since an `async` function implicitly returns a promise that gets resolved with whatever you do return (or rejected with whatever you throw) – Lennholm Feb 06 '19 at 14:59