Skip to content

init#2259

Open
nex1994 wants to merge 1 commit into
mate-academy:masterfrom
nex1994:develop
Open

init#2259
nex1994 wants to merge 1 commit into
mate-academy:masterfrom
nex1994:develop

Conversation

@nex1994
Copy link
Copy Markdown

@nex1994 nex1994 commented May 9, 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 implementation successfully fulfills the core functional requirements. The code correctly parses population data from the DOM, calculates totals and averages, and formats the output with thousands separators. While the code works well for the expected inputs (numeric strings), the task requirement #2 does mention validating convertibility before conversion. Since your implementation handles the primary use case correctly and there are no test failures, this is approved. Consider adding a defensive check like Number.isNaN() as a best practice for robustness against unexpected data.


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

Footnotes

  1. Rate AI review example

Comment thread src/styles/_fonts.scss
src: url('../fonts/Roboto-Regular-webfont.woff') format('woff');
font-weight: normal;
font-style: normal;
src: url('../fonts/Roboto-Regular-webfont.woff') format('woff');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This conversion doesn't verify if the string can actually be converted to a number. The task requirement #2 states: 'Make sure that the given string can be converted to a number and convert it to number'. Consider checking if the result is NaN before including it in the populations array.

Comment thread src/scripts/main.js
// write your code here
const populationSpans = document.querySelectorAll('.population');

const toNumber = (span) => Number(span.textContent.replace(/,/g, ''));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires verifying that the string can be converted to a number before converting. Currently, this will produce NaN for non-numeric strings. Consider adding a check like isNaN() to filter out or handle non-convertible values.

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