Conversation
MykolaFatkullin
commented
Feb 8, 2026
- DEMO LINK
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are a few critical issues that can break the required functionality. Your current parsing logic in src/scripts/main.js (where you replace(/\D/g, '') and then parseInt) can produce '' -> NaN, which will cause the total and average calculation to become NaN; you need to build an array of numbers first, use parseInt(value, 10) or Number(value), and then filter out NaN with something like Number.isFinite before reducing. The average is currently hard-coded to divide by 9, which will be incorrect as soon as the number of .population elements changes; instead, compute average = total / parsedNumbers.length and handle the case where parsedNumbers.length === 0 to avoid division by zero. These two issues directly violate the requirement to “make sure that the given string can be converted to a number and convert it to number” and to calculate the average based on the parsed numeric values.
On the formatting side, using toLocaleString('en-US') forces a specific locale that might not match how the populations are displayed on the page; prefer toLocaleString() with no explicit locale, or detect/match the separator style used in the existing .population spans. Apply the same formatting rule to both the total and the average, and keep assigning to .textContent so you correctly replace the "Calculate it!" placeholder text. Functionally, your DOM selection and write-back logic are solid; once you tighten up numeric validation, remove the hard-coded denominator, and adjust the formatting locale, your solution will fully meet the task requirements. Keep going—you’re very close, and these changes will make your implementation much more robust and correct.
✨ 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
src/scripts/main.js
Outdated
| const total = [...populations] | ||
| .map((population) => population.textContent.replace(/\D/g, '')) | ||
| .map((population) => Number.parseInt(population)) | ||
| .reduce((prev, next) => prev + next, 0); |
There was a problem hiding this comment.
The mapping + reduce pipeline doesn't validate parsed values. Using .replace(/\D/g, '') then Number.parseInt(...) may yield '' -> NaN for some inputs and break the sum. Build an array of parsed numbers, use parseInt(value, 10) or Number(...), then filter out NaN before reducing. This satisfies: "Make sure that the given string can be converted to a number and convert it to number."
src/scripts/main.js
Outdated
| .map((population) => Number.parseInt(population)) | ||
| .reduce((prev, next) => prev + next, 0); | ||
|
|
||
| const average = total / 9; |
There was a problem hiding this comment.
Don't hardcode the divisor 9 for the average. Compute the average from the actual count of parsed numeric values (e.g. const average = total / parsedNumbers.length) and handle the 0 length case to avoid division by zero. This addresses the requirement to calculate the average based on parsed numbers.
src/scripts/main.js
Outdated
| document.querySelector('.total-population').textContent = | ||
| total.toLocaleString('en-US'); |
There was a problem hiding this comment.
Formatting uses toLocaleString('en-US') which forces en-US style. The requirement expects the same thousands-separator style as the populations shown. Either call toLocaleString() without a locale (use runtime locale) or ensure 'en-US' matches the existing population strings; alternatively, detect the separator used in the .population spans and format accordingly.
src/scripts/main.js
Outdated
| document.querySelector('.average-population').textContent = | ||
| average.toLocaleString('en-US'); |
There was a problem hiding this comment.
Same note for the average formatting: ensure the thousands-separator style matches the populations (or omit explicit locale). Also ensure you replace exactly the 'Calculate it!' text by assigning to .textContent (your assignment does this, but adjust formatting as above).
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it meets the core functional requirements: you correctly select all .population elements, safely parse their text into numbers (filtering out invalid values), compute both the total and an average without hard-coding the count, and write the results back into .total-population and .average-population using textContent. Your handling of edge cases like non-numeric values and potentially empty lists is solid, and using parseInt with an explicit radix plus toLocaleString() for display shows good attention to detail.
One non-blocking concern is that toLocaleString() may not always match the exact thousands separator style used in the original page, depending on the browser’s locale; if the task ever requires a strict visual match, you could detect the separator from one of the .population values and format the computed numbers to use the same character. Another minor improvement would be to run npm run lint and fix any reported warnings so your codebase stays consistent with the project’s style expectations. Overall, your implementation is correct, robust for the given requirements, and demonstrates thoughtful handling of input parsing and DOM updates—nice work.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨