Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify usage of promises #4

Merged
merged 1 commit into from
Apr 9, 2021
Merged

Simplify usage of promises #4

merged 1 commit into from
Apr 9, 2021

Conversation

Lioness100
Copy link
Contributor

The getFacts function uses await, .then()/.catch(), new Promise(), and an async function with no await call in it's scope. This can be simplified to using only one method.

Step One

await is not used in this function's scope, only in the new Promise(/* function */) function scope

- async getFact() {
+ getFact() {

Step Two

There's no need to await something if there's nothing after it

- return new Promise(async (resolve, reject) => {
-  await axios
+ return new Promise((resolve, reject) => {
+  axios
    .get(`https://animu.ml/fact`)
    /* more chaining */
});

Step Three

There's no need to wrap axios.get() in a promise, as getFact() will return a promise either way.

- return new Promise((resolve, reject) => {
-   axios
+ return axios
     .get(`https://animu.ml/fact`)
-      .then(function (response) {
-        resolve(response.data);
-      })
-      .catch((err) => {
-         return reject(err);
-       });
+     .then((response) => response.data); 
- });

@NotKyoyo
Copy link
Collaborator

Hello Lioness100,
Thank you for your contribution again. Well, your code is returning several syntax errors.

@Lioness100
Copy link
Contributor Author

Oh boy do I feel stupid 🥲
Could you elaborate on where you see them? My IDE isn't showing anything.

@NotKyoyo
Copy link
Collaborator

Here

@NotKyoyo
Copy link
Collaborator

A sec my bad nvm XD

@Lioness100
Copy link
Contributor Author

No worries lol, sorry if those snippets confused you. I was just trying to show what exactly I changed.

@NotKyoyo
Copy link
Collaborator

Could you please update the version for this PR as well.

@NotKyoyo NotKyoyo merged commit 30676ed into kyrea:master Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants