Added solution#2302
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a correctness issue that can break your total and average calculations. Right now, your map callback returns a number only when the text is numeric and undefined otherwise; this means populationNumbers can contain undefined values, and reduce((sum, num) => sum + num, 0) will eventually compute NaN when it encounters undefined. This violates the requirement that the script reliably computes the total and average from the .population spans, even if some entries are not clean numbers. To fix this, either filter out non-numeric entries before reducing (e.g., using filter(Number.isFinite) or similar) or ensure the mapping always returns a valid numeric value for every .population span, and then use the count of valid numbers when computing the average. Once you address this critical issue, your use of querySelectorAll, toLocaleString for thousands separators, and DOM updates for .total-population and .average-population are all on the right track. After you adjust the numeric handling, your solution will fully meet the functional requirements, and this will be a solid implementation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| const populationNumbers = [...populationSpans].map((span) => { | ||
| const text = span.textContent; | ||
| const cleanedText = text.replace(/,/g, ''); | ||
|
|
||
| if (!isNaN(cleanedText) && cleanedText !== '') { | ||
| return Number(cleanedText); | ||
| } | ||
| }); | ||
|
|
||
| const total = populationNumbers.reduce((sum, num) => sum + num, 0); |
There was a problem hiding this comment.
Your map callback returns Number(cleanedText) only when it is numeric; otherwise it returns undefined. This means populationNumbers can contain undefined values, and reduce((sum, num) => sum + num, 0) will produce NaN when it hits undefined. You should either filter out non-numeric entries before reducing or ensure the mapping always returns a valid number for every .population span.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. You correctly read all .population span texts, safely strip the thousands separators, validate that each value is numeric, and then compute the total and average using only the valid numbers. You also update the .total-population and .average-population spans and format the results with toLocaleString(), which matches the visual style of the original data and fulfills the core requirements. The logic is cleanly contained in main.js, and the previous issue with undefined values breaking the calculations has been resolved by filtering out non-numeric entries before summing. As a minor improvement, you could consider specifying a locale explicitly in toLocaleString() if the project later requires a fixed formatting style, but this is not necessary for correctness. Overall, this is solid, careful work that shows good attention to both robustness and user-facing formatting—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
Added solution, first attempt.