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

APIs week1 #4

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

APIs week1 #4

wants to merge 5 commits into from

Conversation

Veronika121
Copy link
Owner

No description provided.

}
rollTheDices();
// ! Do not change or remove the code below
module.exports = rollTheDices;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your solution should work fine, but if you would like to test your understanding of using functions-returning-a-function, can you fill in the dots in this code snippet?

function rollTheDices() {
  const results = [];

  const pushAndRoll = (dice) => (value) => {
    ...
  };

  rollDice(1)
    .then(pushAndRoll(2))
    .then(pushAndRoll(3))
    .then(pushAndRoll(4))
    .then(pushAndRoll(5))
    .then((value) => {
      results.push(value);
      console.log('Resolved!', results);
    })
    .catch((error) => console.log('Rejected!', error.message));
}

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've doen it. Is it correct?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks correct and I assume it also works as expected. But that is something for you to confirm.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we discussed this code as an alternative to repeating the same code for each .then(). Whether you prefer this DRY version over the repetitive version is a matter of personal choice.

Copy link

@remarcmij remarcmij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Veronika121, looks pretty good. But can you look at my comment for ex4-pokerDiceAll and see if you can make the requested change?

};

getAnonName('John', console.log);
getAnonName('John').then(handleSuccess).catch(handleFailure);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

reject(new Error(`Expected a double digit number but got ${number}`));
}
});
return checkNumber;
}

checkDoubleDigits(11) // should resolve

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


// It looks like the promise gives the value (reject or resolve) immediately after
// the first case (rejection or resolving) happen and doesn't change this value anymore.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is correct. Once a promise is 'settled' (i.e., resolved or rejected) its outcome can no longer be changed. Promises are deliberately designed to work that way.

dices.forEach((dice) => {
resultPromises.push(rollDice(dice));
});
return resultPromises;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try to use .map() here instead of .forEach()? Map dice number to promises? And then return the return the result of Promise.all() (itself a promise) as the return value of this function. With those changes, you do not need to modify line 30 from the way it was.

.then((results) => console.log('Resolved!', results))
.catch((error) => console.log('Rejected!', error.message));

//1. Dices that have not yet finished their roll continue to do so because reject() doesn't mean 'return'.
//2. There only a single call to either .then() or .catch() despite resolve() and/or reject() being called more than once
//because .then() or .catch() can be called only ones. But I'm not sure :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a promise becomes settled it must have either been resolved or rejected. It can only be resolved or rejected once. Therefore .then() or .catch() is called only once here. Additional calls to resolve() and reject() have no effect after that any more.

}
rollTheDices();
// ! Do not change or remove the code below
module.exports = rollTheDices;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we discussed this code as an alternative to repeating the same code for each .then(). Whether you prefer this DRY version over the repetitive version is a matter of personal choice.

@Veronika121
Copy link
Owner Author

@remarcmij Thank you for your feedback! I've changed ex4.

Copy link

@remarcmij remarcmij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can now approve your PR.

}

Promise.all(rollTheDices())
rollTheDices()
.then((results) => console.log('Resolved!', results))
.catch((error) => console.log('Rejected!', error.message));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

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