rizan_ibrahim-w2-UsingApis #66
Conversation
|
|
||
| }) | ||
| .catch((error) => { | ||
| throw new Error(`Network error! ${error.message}`); |
There was a problem hiding this comment.
There is no point in catching the error here if the only thing you are doing is throwing another one. You might as well get rid of the try/catch block and let the original error (the one you are catching here) be caught by the try/catch block of the main() function.
Also, since this week we are focusing on async/await can you use this instead of using then()/catch()? That is what you are using in the main() function. It is usual to stick to one way of doing things in a project.
By the way, your VSCode setup is not formatting the code entirely as expected. I would expect the code to be automatically formatted like this:
function requestData(url) {
return fetch(url)
.then((response) => {
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
return response.json();
})
.catch((error) => {
throw new Error(`Network error! ${error.message}`);
});
}| renderError(error); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Apart from my comments above your code works as expected.
| document.body.appendChild(img); | ||
|
|
||
|
|
||
| fetchAndPopulatePokemons(); |
There was a problem hiding this comment.
I'm missing the Get Pokemon button from the assignment description that the user should click to fetch the pokemons.
Use fetchData() to load the pokemon data on demand (i.e. when the button is pressed) from the public API and populate the <select> element in the DOM.
|
|
||
| const img = document.createElement('img'); | ||
| img.id = 'pokemon-img'; | ||
| img.alt = 'Pokemon Image'; |
There was a problem hiding this comment.
It's nice that you set the .alt attribute but at present it is not very informative. Is there a better description that you could use for the currently selected pokemon in the pokemon object returned by the API?
| img:hover { | ||
| transform: scale(1.05); | ||
| box-shadow: 0 8px 16px rgba(0, 0, 0, 0.15); | ||
| } |
| } | ||
|
|
||
|
|
||
| window.addEventListener('load', main); |
There was a problem hiding this comment.
Apart from the missing button, the code works fine.
| } | ||
| return value; | ||
| }; | ||
| async function main() { |
There was a problem hiding this comment.
Nice solution! But for readability, it is best to separate function definitions with a blank line.
| /* | ||
| When you use Promise.race(), it’s like starting a bunch of races at once and declaring a winner as soon as the first one finishes—whether it succeeds or fails. But here’s the catch: even after that first result comes in, the other promises you passed to race() don’t just disappear. They keep chugging away in the background because JavaScript doesn’t have a built-in way to halt promises once they’ve started. Imagine rolling five virtual dice with Promise.race()—the moment the first die lands, you get your result, but the other four? They’re still spinning, and there’s no way to magically freeze them mid-air. | ||
|
|
||
| If you really need to stop those leftover promises (like saving resources or avoiding unnecessary work), things get trickier. You’d have to roll up your sleeves and use tools like AbortController to manually signal cancellation, or come up with your own cleanup logic. It’s doable, but it’s definitely not as simple as just calling Promise.race() and calling it a day.*/ |
There was a problem hiding this comment.
This looks like an AI-generated explanation. 😞
There was a problem hiding this comment.
yeah i i didnt know how to express exactly but i will change it thanks for ur attention
| console.log(`Death: ${death.date}, ${death.place.locationString}`); | ||
| if (birth) { | ||
| console.log( | ||
| `Birth: ${birth?.date || 'Unknown'}, ${birth?.place?.locationString || 'Unknown'}` |
There was a problem hiding this comment.
There is still a bug to be solved with the location. Currently it prints as [object Object]. This is for both the browser version and the Node version.
| } catch (error) { | ||
| throw new Error(`Network error! ${error.message}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
The requestData() is expected to return a promise that resolves to a JavaScript (from response.json()) or to return a rejected promise in case of network errors or HTTP errors. That is what your code does correctly. However, you don't need a try/catch block here. The same behaviour can be accomplished with this simpler code:
async function requestData(url) {
const response = await fetch(url);
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
return await response.json();
}| @@ -39,6 +39,8 @@ async function fetchAndPopulatePokemons() { | |||
| try { | |||
| const data = await fetchData(url); | |||
| const select = document.querySelector('select'); | |||
| select.innerHTML = ''; | |||
There was a problem hiding this comment.
After you press the button the select box shows the name of the first pokemon (bulbasaur) but does not show its image. A solution could be to either show the bubasaur image or add a placeholder for the first option in the select box before you add the pokemon options. For instance:
const option = document.createElement('option');
option.value = '';
option.textContent = 'Select a Pokemon';
option.disabled = true;
option.selected = true;
select.appendChild(option);I adapted this from StackOverflow: https://stackoverflow.com/questions/5805059/how-do-i-make-a-placeholder-for-a-select-box
| When you use Promise.race(), it’s like starting a bunch of races at once and declaring a winner as soon as the first one finishes—whether it succeeds or fails. But here’s the catch: even after that first result comes in, the other promises you passed to race() don’t just disappear. They keep chugging away in the background because JavaScript doesn’t have a built-in way to halt promises once they’ve started. Imagine rolling five virtual dice with Promise.race()—the moment the first die lands, you get your result, but the other four? They’re still spinning, and there’s no way to magically freeze them mid-air. | ||
|
|
||
| If you really need to stop those leftover promises (like saving resources or avoiding unnecessary work), things get trickier. You’d have to roll up your sleeves and use tools like AbortController to manually signal cancellation, or come up with your own cleanup logic. It’s doable, but it’s definitely not as simple as just calling Promise.race() and calling it a day.*/ | ||
| When you use Promise.race(), it starts all the promises at once and gives you the result of the first one that done, regardless of whether it resolved or rejected, However the other promises don’t stop—they keep running in the background. If you need to cancel those extra promises, you'd have to use something like AbortController or add your own logic. It’s not as simple as just calling Promise.race()*/ |
There was a problem hiding this comment.
It's okay to seek help using AI so long as you understand what it says and learn from it.
No description provided.