Feras Aldmeashki-w2-usingAPIs #69

Feras-ALdemashki · 2025-02-04T20:24:50Z

No description provided.

remarcmij

Hi @Feras-ALdemashki, I have identified some issues for you to address. The major one is in the pokemon exercise, where you should only fetch the pokemons after pressing the button. The other issues are minor. Please have a look at my comments below.

remarcmij · 2025-02-12T12:22:01Z

3-UsingAPIs/Week2/assignment/ex1-programmerFun/index.js

@@ -17,28 +19,31 @@ Full description at: https://github.com/HackYourFuture/Assignments/blob/main/3-U
   should result in a network (DNS) error.
 ------------------------------------------------------------------------------*/
 function requestData(url) {
-  // TODO return a promise using `fetch()`
+  return fetch(url).then((Response) => {


Since this week is about using async/await, can you refactor this to use that?

Variable names should start with a lowercase letter (camelcase): Response -> response.

Don't forget to check response.ok.

remarcmij · 2025-02-12T12:27:07Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

-  // TODO complete this function
+async function main() {
+  try {
+    const response = await fetchData(


You should fetch the pokemons only when the button is pressed, not at immediately as you are doing here, see the instructions:

Use fetchData() to load the pokemon data on demand (i.e. when the button is pressed) from the public API and populate the element in the DOM.

Hello Jim, i did all the other changes but I'm not sure if i understand this one I'm getting the same result in the description . As now i fetch the data and when the button is clicked i append it to the drop down isn't the same ? or I'm missing something

You are fetching too soon. You should fetch only when the button is pressed (i.e. on demand) because that was how it was specified in the assignment description.

Use fetchData() to load the pokemon data on demand (i.e. when the button is pressed)

i changed the solution now the fetchData( ) function will work when the button is clicked

remarcmij · 2025-02-12T12:30:09Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

+  } catch (error) {
+    throw new Error('fetch failed');
+  }


There is no added value in catching an error here if the only thing that you are doing is rethrowing another error. Not every function that uses async/await needs a try/catch block if there already is a try/catch block higher up in the call chain that handles the error.

remarcmij · 2025-02-12T12:32:49Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

+    const response = await fetch(pokemonUrl);
+    const pokemonData = await response.json();


Why not call he reusable fetchData() function here that already deals with checking response.ok etc.

You didn't address this point.

remarcmij · 2025-02-12T12:35:12Z

3-UsingAPIs/Week2/assignment/ex3-rollAnAce.js

+  } catch (error) {
+    throw new Error(error);
+  }


Useless try/catch here. You already have a try/catch block in the calling main() function.

remarcmij · 2025-02-12T12:41:38Z

3-UsingAPIs/Week2/assignment/ex4-diceRace.js

+    const result = await rollDice();
+    console.log('Resolved!', results);


When I run your code I get an undefined:

Rejected! results is not defined

Can you spot the typo?

remarcmij · 2025-02-12T12:41:52Z

3-UsingAPIs/Week2/assignment/ex4-diceRace.js

 }

 // ! 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.
+// promise.race does not cancel the other promises, the other promises will keep going but we will not get their results as we get the first promise result


remarcmij · 2025-02-12T12:45:33Z

3-UsingAPIs/Week2/assignment/ex5-vscDebug.js

+  const birthPlace = birth?.place?.locationString?.en || 'Unknown';
+  console.log(`Birth: ${birthDate}, ${birthPlace}`);
+
+  const deathDate = death?.date || 'unknown';


While checking for undefined properties and replacing it with unknown is a good strategy in general, in the case of the death.date not being defined this usually means that the person is not dead. By using unknown you are leaving the possibility open that the person might be dead. That might not always be appreciated 😉 .

remarcmij · 2025-02-12T12:46:30Z

3-UsingAPIs/Week2/assignment/ex5-vscDebug.js

  } catch (err) {
-    console.error(`Something went wrong: ${err.message}`);
+    console.error(err);


Why did you think this change was appropriate?

remarcmij · 2025-02-12T12:49:43Z

3-UsingAPIs/Week2/assignment/ex6-browserDebug/index.js

+  );
+
+  if (death) {
+    addTableRow(


This is a better approach to dealing with an undefined death property.

…is fetched

remarcmij

Hi @Feras-ALdemashki, thanks for the updates. I still have some comments but they are minor therefore I'm happy to approve your PR now.

remarcmij · 2025-02-12T16:12:25Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

+  } catch (error) {
+    console.error(error);
+  }


If fetch() throws an error you get it here and print an error message on the console. But after that the async fetchData() function returns a promise that resolves to the value undefined. It doesn't return a rejected promise because you have already "consumed" the error here. Therefore, the catch clause of the button event handler will not be triggered.

It is better to get rid of the try/catch blocks here and let the calling function handle the error (in your case, the event handler of the button element).

remarcmij · 2025-02-12T16:14:34Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

+      const pokemons = await fetchData(
+        'https://pokeapi.co/api/v2/pokemon?limit=151'
+      );
+      pokemons.forEach((pokemon) => {


If an error occurs in fetchData() the pokemons variable will be set to undefined, because you have already "consumed" the error in fetchData().

remarcmij · 2025-02-12T16:15:06Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

+    const response = await fetch(pokemonUrl);
+    const pokemonData = await response.json();


You didn't address this point.

remarcmij · 2025-02-12T16:18:47Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

 }

 function main() {
-  // TODO complete this function
+  fetchAndPopulatePokemons();


It would seem more logical (given the name fetchAndPopulatePokemons() to only call this function when you are actually doing what the name suggests. Tha would imply that you only call that function when the button is pressed. It would require that move the event handler for the button the main() function.

remarcmij · 2025-02-12T16:20:31Z

3-UsingAPIs/Week2/assignment/ex2-pokemonApp/index.js

+  document.body.appendChild(selectElement);
+
+  buttonElement.addEventListener('click', async () => {
+    selectElement.innerHTML = '';


Excellent to clear out the existing content before appending new content. Otherwise the list will get longer and longer if the button is pressed more than once.

Feras-ALdemashki added 2 commits February 3, 2025 15:12

finshed ex1 to ex4

73c9b3e

solved ex5 and ex6

c7c2e76

remarcmij self-assigned this Feb 5, 2025

remarcmij requested changes Feb 12, 2025

View reviewed changes

remarcmij added the Needs work label Feb 12, 2025

Feras-ALdemashki added 4 commits February 12, 2025 14:31

changing therequest data function to async in ex1

0a037d6

fixing the typo in ex4

01b2bea

fixing the error message in ex5

26d7442

changed the solution for ex2 now when the button i clicked the data …

0bd7670

…is fetched

remarcmij approved these changes Feb 12, 2025

View reviewed changes

remarcmij added Approved and removed Needs work labels Feb 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feras Aldmeashki-w2-usingAPIs #69

Feras Aldmeashki-w2-usingAPIs #69

Feras-ALdemashki commented Feb 4, 2025

remarcmij left a comment

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

Feras-ALdemashki Feb 12, 2025

remarcmij Feb 12, 2025

Feras-ALdemashki Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij left a comment

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

remarcmij Feb 12, 2025

		const response = await fetch(pokemonUrl);
		const pokemonData = await response.json();

		const result = await rollDice();
		console.log('Resolved!', results);

Feras Aldmeashki-w2-usingAPIs #69

Are you sure you want to change the base?

Feras Aldmeashki-w2-usingAPIs #69

Conversation

Feras-ALdemashki commented Feb 4, 2025

remarcmij left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remarcmij left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment