Conversation
Yevhen-Srdk
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 some core requirements around validation and average calculation are not yet met, which can lead to incorrect results or NaN in the UI. In src/scripts/main.js, you currently convert values with Number(item.textContent.replace(/,/g, '')) and sum them directly; the task explicitly requires checking that each span’s text can be converted to a number, so you should store the result in a variable (e.g., const value = Number(...)) and only include it if Number.isFinite(value) (or a similar check) passes, while also trimming whitespace and handling commas/non-breaking spaces. You also need to keep a separate counter of how many entries were successfully parsed, and compute the average from that counter instead of population.length, with a guard for the case where the valid count is zero to avoid division by zero and decide what to show when no valid numbers are found. Additionally, please rename the typo avaragePopulation to averagePopulation for clarity and consistency. Once you add proper numeric validation, base the average on the number of valid items, and handle the zero-valid case, your solution will align well with the task’s requirements, since the rest of the logic and formatting is already on the right track.
✨ 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
| for (const item of population) { | ||
| totalPopulationCount += Number(item.textContent.replace(/,/g, '')); |
There was a problem hiding this comment.
You convert text with Number(item.textContent.replace(/,/g, '')) but do not verify the result. The task requires ensuring the string can be converted to a number — parse into a variable and check it (e.g., const value = Number(...); if (Number.isFinite(value)) { ... }). Also keep a count of successfully parsed entries to use for the average.
src/scripts/main.js
Outdated
| const averagePopulationResult = Math.round( | ||
| totalPopulationCount / population.length, | ||
| ); |
There was a problem hiding this comment.
Average is calculated using population.length. According to the requirements the average must be based on the parsed numbers, so use the count of valid parsed values instead. Also add a guard for zero valid entries to avoid division by zero (decide what to display when no valid numbers are found).
src/scripts/main.js
Outdated
| ); | ||
|
|
||
| const totalPopulation = document.querySelector('.total-population'); | ||
| const avaragePopulation = document.querySelector('.average-population'); |
There was a problem hiding this comment.
Minor but helpful: const avaragePopulation = ... has a typo. Rename the variable to averagePopulation to avoid confusion.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still two functional issues that block fully correct behavior. First, in src/scripts/main.js, the parsing currently only removes commas from the .population text; it does not trim or handle non-breaking spaces, so values like "1�,234" or strings with extra whitespace may fail Number(...) and be skipped even though they are valid numbers. Please normalize the raw text before conversion, for example by removing commas, non-breaking spaces (\u00A0), and general whitespace, then passing the cleaned string into Number(...) so Number.isFinite reliably identifies valid values. Second, the average calculation divides by isNumberCount without guarding against the case where no valid numbers are found, which can produce NaN or Infinity in the UI; instead, compute the average only when isNumberCount > 0, and decide on a clear fallback (like 0 or leaving the placeholder text) when it is zero. Once those are fixed, your use of querySelectorAll('.population'), counting only valid numeric entries, and formatting with toLocaleString('en-US') otherwise looks solid and close to meeting all requirements. You are very close—treat these as final polish steps to make your solution robust in all edge cases.
✨ 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
| let isNumberCount = 0; | ||
|
|
||
| for (const item of population) { | ||
| const value = Number(item.textContent.replace(/,/g, '')); |
There was a problem hiding this comment.
Parsing currently strips only commas. The task requires trimming and handling non-breaking spaces and other whitespace before converting. Consider normalizing the raw text first, e.g. const raw = item.textContent.replace(/[\u00A0,\s]/g, ''); const value = Number(raw); so Number.isFinite(value) reliably detects valid numbers.
src/scripts/main.js
Outdated
| const averagePopulationResult = Math.round( | ||
| totalPopulationCount / isNumberCount, | ||
| ); |
There was a problem hiding this comment.
Guard against zero valid entries before computing the average to avoid division by zero. Compute average only when isNumberCount > 0, otherwise decide what to display (for example 0 or keep the original placeholder). Example: const averagePopulationResult = isNumberCount > 0 ? Math.round(totalPopulationCount / isNumberCount) : 0;
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because the main script correctly finds all .population spans, normalizes their text (removing commas, non-breaking spaces, and extra whitespace), converts to numbers safely, and then computes both the total and average using only valid numeric values. Both results are written back into .total-population and .average-population with toLocaleString('en-US'), which satisfies the thousands-separator requirement and matches the task’s formatting expectations.
There are a couple of process and repo-structure details to improve: the task asked you to only change src/scripts/main.js, but you also modified README.md, so you should revert that extra change or clearly justify it in a future PR [CHECKLIST ITEM #4]; also, remember to run npm run lint and fix any reported issues so your code style is consistent with the project [CHECKLIST ITEM #13]. This approval is an educational override to help you keep progressing while you practice following all task constraints more strictly. Overall, your handling of normalization, numeric validation, and safe average calculation (guarding against zero valid entries) is solid—nice work, and keep refining those workflow details like limiting changed files and running lint regularly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨