Salih-w1-UsingAPIs #58
Conversation
There was a problem hiding this comment.
Hi @rektozarius, thanks for your PR. Well done. I have some minor comments below but I'm happy to approve your PR as it stands now.
| return new Promise((resolve, reject) => { | ||
| setTimeout(() => { | ||
| if (!firstName) { | ||
| reject(new Error("You didn't pass in a first name!")); |
There was a problem hiding this comment.
Since there is nothing more to do after calling reject() in the setTimeout() callback function, it is best to add a return after reject() to exit early from the callback. In this case, the test is still passing because calling resolve() after reject() does not change the outcome of the promise from rejected to fulfilled.
| reject(new Error("You didn't pass in a first name!")); | |
| reject(new Error("You didn't pass in a first name!")); | |
| return; |
| export function checkDoubleDigits(number) { | ||
| return new Promise((resolve, reject) => { | ||
| if ((number >= 10) && (number <= 99)) { | ||
| resolve("This is a double digit number!"); |
There was a problem hiding this comment.
For the same reason as the previous exercise:
| resolve("This is a double digit number!"); | |
| resolve("This is a double digit number!"); | |
| return; |
|
|
||
| // Use callback to notify that the die rolled off the table after 6 rolls | ||
| if (roll >= 6) { | ||
| reject(new Error('Oops... Die rolled off the table.')); |
There was a problem hiding this comment.
Note that here I do not expect a return after reject(). This is because in the assignment description it was asked to let the die finish its scheduled rolls, even if it rolls off the table.
| console.log(`Success! Die settled on ${value}.`); | ||
| } | ||
| }); | ||
| const rollo = rollDie() |
There was a problem hiding this comment.
Can you explain the purpose of the rollo constant?
There was a problem hiding this comment.
I think it was from an earlier version of the code. I was probably trying something else, and forgot to remove it. It will be removed in the latest commit.
| If the promise did not resolve before being returned, it is still in a pending state and | ||
| the only outcome possible is being rejected because of the way we constructed the function. | ||
| If the promise resolved before being returned, then we would only see the success message and | ||
| there would be no errors to catch. |
There was a problem hiding this comment.
That's a whole lot of words. However, I see in the code only two possibilities:
- The die completes its rolls without rolling off the table and
resolve()is called before the function returns. - The die falls off the table and
reject()is called. Then it completes its schedules while off the table,resolve()is called and then the function exits.
There is no scenario where the function exits with the promise still pending.
The problem with the callback version does not occur with the promise version because in scenario 2 above, the call the resolve() does not change the rejected outcome of the promise that was settled by an earlier call to reject().
There was a problem hiding this comment.
Thanks for the comments Jim. It is more clear now with your explanation. The session on last Sunday helped a lot as well.
| only when all the promises inside the promise array resolves. If one of them rejects, then Promise.all() | ||
| rejects immediately. Since promises are returned by asynchronous operations, Promise.all() doesn't stop | ||
| pending promises from trying to settle even if the returned promise is rejected. | ||
| That's why we see the unfinished dice continue to roll. |
No description provided.