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

rizan_ibrahim-UsingAPIs-w1-assingment #57

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

Conversation

rizan-ibrahim
Copy link

No description provided.

@rizan-ibrahim rizan-ibrahim changed the title modified rizan_ibrahim-UsingAPIs-w1-assingment Jan 29, 2025
@remarcmij remarcmij self-assigned this Jan 30, 2025
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 @rizan-ibrahim, there is still one TODO left to be done, but otherwise your code looks fine. But please include the test reports for your next PR (week 2), so that I can quickly see the result without having to run the test myself.

Comment on lines 28 to 30
getAnonName('John')
.then(console.log)
.catch(console.log);

Choose a reason for hiding this comment

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

While your solution is fine, it looks like something is wrong with your VSCode setup, because it should automatically format your file such that the .then() and .catch() method are indented by two spaces:

Suggested change
getAnonName('John')
.then(console.log)
.catch(console.log);
getAnonName('John')
.then(console.log)
.catch(console.log);

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

Choose a reason for hiding this comment

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

👍

.catch((error) => console.log( error.message ))



}

// ! Do not change or remove the code below

Choose a reason for hiding this comment

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

There is still a TODO left to be done.

/*
Explanation:
When using `Promise.all`, if one of the promises is rejected, the entire `Promise.all` is rejected immediately with that reason. However, the other promises that have not yet finished their roll will continue to execute. This is because `Promise.all` does not cancel the remaining promises; it simply stops waiting for them once one promise is rejected.
*/

Choose a reason for hiding this comment

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

Excellent explanation. This is exactly right.

.then((value) => {
results.push(value);
return rollDie(5);
})

Choose a reason for hiding this comment

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

👍

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 @rizan-ibrahim, thanks for the update. Please check my refinement of your explanation for ex3. I will approve this PR now.

/*
Explanation:
The `rollDie` function simulates rolling a die until it lands on a value of 6. It uses a recursive function `rollOnce` to keep rolling the die. The function returns a promise that resolves when the die lands on 6 and rejects if an error occurs. The `rollOnce` function logs each attempt and rolls the die again after a 1-second delay if the value is not 6.
*/

Choose a reason for hiding this comment

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

This explains what the promise version does, not why the behaviour is different from the callback version of the code. The key point here is that when a die rolls off the table if calls reject() and then continues its scheduled rolls while off the table. When it has completed its scheduled rolls it finally calls resolve(). But contrary to the callback version this has no effect because once a promise is settled, either by a call to reject() or to resolve(), subsequent calls to either of them are ignored.




.catch((error) => console.log(error.message));

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants