-
Notifications
You must be signed in to change notification settings - Fork 10
Ilias_khugaev_W1UsingAPIs #39
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
base: main
Are you sure you want to change the base?
Ilias_khugaev_W1UsingAPIs #39
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Narpimers, all exercises are done correctly, good work. Note that the test reports for the Browser module are not expected to be part of this PR.
Contact me with a DM on Slack to fix your VSCode Prettier setup. It doesn't to work as expected.
if (!firstName) { | ||
reject(new Error("You didn't pass in a first name!")) | ||
} else { | ||
const fullName = `${firstName} Doe`; | ||
resolve(fullName) | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
-
You can also "exit early" when rejecting the promise. The idea is to check for errors at the start of the function body and return "early" in case of errors. This eliminates the need for an
else
clause and also requires one less indentation level on the remainder of the code. For a short function like the one here it does not make much of a difference but when you function becomes larger it will improve readability. -
Your VSCode Prettier setup is not working correctly as the code is not automatically formatted according to the Prettier rules. Send me a DM on Slack so we can fix that for you.
if (!firstName) { | |
reject(new Error("You didn't pass in a first name!")) | |
} else { | |
const fullName = `${firstName} Doe`; | |
resolve(fullName) | |
}; | |
if (!firstName) { | |
reject(new Error("You didn't pass in a first name!")); | |
return; | |
} | |
const fullName = `${firstName} Doe`; | |
resolve(fullName); | |
}); |
} else { | ||
reject(new Error(`Expected a double digit number but got ${number}`)) | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
} | ||
|
||
// ! Do not change or remove the code below | ||
if (process.env.NODE_ENV !== 'test') { | ||
main(); | ||
} | ||
|
||
// TODO Replace this comment by your explanation that was asked for in the assignment description. | ||
// In a promise, if there is a reject, it cannot be resolved, and vice versa. If there is a resolve, there cannot be a reject. | ||
// There is no such limitation with callbacks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you got the idea. But to be more precise, a promise settles on the first call on either resolve()
or reject()
. Subsequent calls to resolve()
or reject()
do not change the already settled outcome of the promise.
In this example, in the case that a die rolls off the table the reject()
function is called. Then when that same die completes its scheduled rolls the resolve()
function is called, but that doesn't change the promise from rejected to resolved. As you rightly point out, this is different from the callback version.
return rollDie(dice); | ||
}); | ||
return Promise.all(diceArray);; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct!
Note that you can also use the shorter notation for an arrow function if the function body consist of one statement only, like this:
export function rollDice() {
const dice = [1, 2, 3, 4, 5];
const diceArray = dice.map((die) => rollDie(die));
return Promise.all(diceArray);
}
(BTW: the singular form for "dice" is "die". Since the callback function of map()
takes one element of the array at a time you should use the singular form for the argument name as I did in the example above).
You can shorter the code further by passing the rollDie
function itself as the map()
parameter:
export function rollDice() {
const dice = [1, 2, 3, 4, 5];
const diceArray = dice.map(rollDie);
return Promise.all(diceArray);
}
Finally, you might as well get rid of the intermediate array like this:
export function rollDice() {
const dice = [1, 2, 3, 4, 5];
return Promise.all(dice.map(rollDie));
}
|
||
// TODO: expand the chain to include five dice | ||
return rollDie(1) | ||
let index = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable seems to be unused.
.then((value) => { | ||
results.push(value); | ||
return results; | ||
}); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.