Skip to content

feat: calculate average and total value-based to parsed numbers#2244

Open
linska wants to merge 1 commit into
mate-academy:masterfrom
linska:dev
Open

feat: calculate average and total value-based to parsed numbers#2244
linska wants to merge 1 commit into
mate-academy:masterfrom
linska:dev

Conversation

@linska
Copy link
Copy Markdown

@linska linska commented Apr 20, 2026

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your solution is approved because it meets all the core functional requirements of the task. You correctly gather the text from the .population spans, strip thousands separators, convert the values to numbers (skipping invalid entries), compute both total and average, and then format and display those results in .total-population and .average-population using toLocaleString('en-US'). The logic is clear, concise, and stays confined to src/scripts/main.js as required, so there are no critical blockers for functionality.

There are a few non-blocking improvements you might consider for robustness and style:

  • Add null checks before writing to .total-population and .average-population so the code fails gracefully if those elements are missing.
  • Prefer textContent over innerHTML when inserting plain numbers to avoid any unintended HTML parsing.
  • If you expect noisier input, consider sanitizing population strings with something like text.replace(/[^\d.-]/g, '') before converting to Number.

Overall, this is a solid implementation that fulfills the task, and refining these small details will make your code even more resilient and idiomatic.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants