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

Konjit-w2-UsingAPIs #59

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

Conversation

konjit
Copy link

@konjit konjit commented Feb 3, 2025

Solutions for UsingAPIs W2 Assignment

@ddoyediran ddoyediran self-assigned this Feb 9, 2025
// TODO return a promise using `fetch()`
return fetch(url)
.then((response) => {
if (!response.ok) {

Choose a reason for hiding this comment

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

Nice job checking if the response wasn't ok.

.catch((error) => {
renderError(error);
});
async function main() {

Choose a reason for hiding this comment

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

Nice use of Async Await here.

@@ -21,18 +21,79 @@ Use async/await and try/catch to handle promises.
Try and avoid using global variables. As much as possible, try and use function
parameters and return values to pass data back and forth.
------------------------------------------------------------------------------*/
function fetchData(/* TODO parameter(s) go here */) {

Choose a reason for hiding this comment

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

Great job completing Exercise 2 ex2-pokemonApp.

One thing, I think we can try to improve this week is writing meaningful Functions and Variable Names in programming. The book Clean Code by Robert Martin helped me. You don't have to buy, but you can check this article about how to write good variables and functions name.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the info will check the article.


export async function rollDieUntil(desiredValue) {
let value = await rollDie();
while (value !== desiredValue) {

Choose a reason for hiding this comment

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

Well done changing this from recursion to using while loop.

}

// ! 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.
// All created promises will run independently (concurrent manner). In this case Promise.race waits for

Choose a reason for hiding this comment

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

Correct. Well done. Promise.race() resolves as soon as the first promise in the array resolves, but it does not cancel or stop the other promises. They continue executing independently

@@ -10,19 +10,27 @@ async function getData(url) {

function renderLaureate({ knownName, birth, death }) {

Choose a reason for hiding this comment

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

I got the following error "Something went wrong: fetch is not defined" on my terminal when I run command node ex5-vscDebug.js. Note: we are using fetch() method but we don't have access to it.

Hint: Can we try to use a fetch package like https://www.npmjs.com/package/node-fetch or find another way to import and access fetch()?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you so much for taking the time to review. When I run node ex5-vscDebug.js in my terminal, it works without importing const fetch = require('node-fetch');. Is it including the node-fetch considered a good practice even if it works without it?

Choose a reason for hiding this comment

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

I see. Then it is probably an error on my end. You don't have to. I suggested const fetch = require('node-fetch'); because we are using fetch and we need a way to import it since it wasn't imported.

@@ -30,8 +30,22 @@ function renderLaureate(ul, { knownName, birth, death }) {
const li = createAndAppend('li', ul);

Choose a reason for hiding this comment

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

Work as expected. Well done.

### 3-UsingAPIs - Week2

| Exercise | Passed | Failed | ESLint |
|-------------------|--------|--------|--------|

Choose a reason for hiding this comment

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

@konjit Well done completing Week 2 Homework. Your solution looks great. I left feedback on things we can improve and for exercise 5. Let me know if you have any questions.

@ddoyediran
Copy link

Approved now. Well done.

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