Skip to content

added solution#2188

Open
TiFAu wants to merge 3 commits intomate-academy:masterfrom
TiFAu:develop
Open

added solution#2188
TiFAu wants to merge 3 commits intomate-academy:masterfrom
TiFAu:develop

Conversation

@TiFAu
Copy link

@TiFAu TiFAu commented Feb 19, 2026

No description provided.

Copy link

@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

I am requesting changes because there are a few critical issues that break the stated requirements and can cause incorrect behavior. In src/scripts/main.js, the code reads values with innerHTML and sums them directly; this can include stray HTML/whitespace and, more importantly, there is no check that each cleaned string is actually convertible to a number, so any invalid entry will turn the total and average into NaN (violating the “ensure strings are convertible to numbers” requirement). When computing the average, you divide by spanPopulation.length instead of by the count of successfully parsed numeric values, which means the average will be wrong if any entries are invalid or skipped. The result is then written using averagePopulation.outerHTML = ... and totalPopulation.outerHTML = ..., which replaces the entire span elements (including their classes) instead of just replacing the "Calculate it!" text inside them, directly contradicting the requirement to keep the spans and only update their contents; you also do not guard against null when these elements are missing.

To fix this:

  • Read values with span.textContent.trim(), strip separators (e.g. .replace(/[\,\s]/g, '')), convert with Number(...), and filter out non-finite values using Number.isFinite.
  • Build an array of valid numbers, compute the total with reduce, and compute the average using validNumbers.length while guarding against division by zero if there are no valid numbers.
  • Before writing, check if (averagePopulation) { ... } and if (totalPopulation) { ... }, then set only element.textContent = formattedValue so the spans and their classes remain intact. Once you address these points, your overall approach to selecting the nodes, doing the calculations, and formatting the numbers will align well with the task requirements, and the solution will be robust and maintainable.

✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

'use strict';

// write your code here
const spanPopulation = [...document.querySelectorAll('.population')];

Choose a reason for hiding this comment

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

Selecting the elements this way is okay, but when you later read values prefer textContent (and .trim()) instead of innerHTML to get raw text without accidental HTML or extra whitespace.


// write your code here
const spanPopulation = [...document.querySelectorAll('.population')];
const averagePopulation = document.querySelector('.average-population');

Choose a reason for hiding this comment

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

No null-check for averagePopulation. If the element is missing document.querySelector returns null and later assignment will throw — guard before writing to it.

// write your code here
const spanPopulation = [...document.querySelectorAll('.population')];
const averagePopulation = document.querySelector('.average-population');
const totalPopulation = document.querySelector('.total-population');

Choose a reason for hiding this comment

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

No null-check for totalPopulation. Add an existence check before assigning to avoid runtime errors when element is missing.

Comment on lines 6 to 9
const total = spanPopulation.reduce(
(sum, n) => sum + Number(n.innerHTML.split(',').join('')),
0,
);

Choose a reason for hiding this comment

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

Using reduce with Number(n.innerHTML.split(',').join('')) will produce NaN if any value is not a valid number and then the whole total becomes NaN. Instead, clean each span's text (use textContent.trim()), remove separators (e.g. .replace(/[,\s]/g, '')), parse with Number or parseInt, check !Number.isNaN(value)/Number.isFinite(value), collect valid numbers, and reduce over that array.

const averagePopulation = document.querySelector('.average-population');
const totalPopulation = document.querySelector('.total-population');
const total = spanPopulation.reduce(
(sum, n) => sum + Number(n.innerHTML.split(',').join('')),

Choose a reason for hiding this comment

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

This line specifically uses innerHTML and a naive split(',').join('') approach. Prefer n.textContent.trim().replace(/[,\s]/g, '') and validate the cleaned string before converting — the requirement explicitly asks to ensure the string can be converted to a number.

(sum, n) => sum + Number(n.innerHTML.split(',').join('')),
0,
);
const average = Math.round(total / spanPopulation.length);

Choose a reason for hiding this comment

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

Average is computed as total / spanPopulation.length. The requirement says to calculate based on parsed numbers — the denominator should be the count of successfully parsed numeric values, and you must guard against division by zero (empty or all-invalid inputs).


function convertNumbers(number) {
const resultNumber = [];
const numberToString = String(number);

Choose a reason for hiding this comment

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

convertNumbers assumes the input is a valid integer string; if number is NaN or not an integer the output will be wrong. Ensuring earlier validation (only calling this with finite integers) will fix this — or consider using Intl.NumberFormat for robustness.

return resultNumber.join(',');
}

averagePopulation.outerHTML = convertNumbers(average);

Choose a reason for hiding this comment

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

Setting outerHTML replaces the entire element including its tag and classes, which breaks the requirement to replace the "Calculate it!" text inside the span. Use averagePopulation.textContent = ... and totalPopulation.textContent = ... and only if those nodes exist.

}

averagePopulation.outerHTML = convertNumbers(average);
totalPopulation.outerHTML = convertNumbers(total);

Choose a reason for hiding this comment

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

Same issue as above: totalPopulation.outerHTML = ... removes the element. Set textContent and check if (totalPopulation) { ... } first.

Copy link

@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

I am requesting changes because the current main.js implementation still violates key functional requirements and can produce incorrect results. The summation uses Number(n.innerHTML.split(',').join('')) without validating the parsed value, so any non-numeric span text will turn the total into NaN and break the calculation, contrary to the requirement to ensure strings are convertible to numbers before summing. You should instead read with n.textContent.trim(), strip separators like commas/spaces (e.g. `.replace(/[,

]/g, '')), convert with Number(...), and build a validNumbersarray that only keepsNumber.isFinite(value)entries, then reduce over that array. The average is currently computed using the originalspanPopulation.length; it must be calculated as Math.round(total / validNumbers.length)and guarded so that ifvalidNumbers.length === 0you show0or keep "Calculate it!" rather than dividing by zero. In addition,convertNumbersshould only be called with validated finite numbers (including the rounded average) to avoid formattingNaNorInfinity, and when writing results you should check that the result elements exist and set only their textContent, not any HTML structure. Finally, the task explicitly said to change only main.js`, but style files were modified as well; those unrelated style changes need to be reverted for this submission to fully comply with the requirements. Once these issues are fixed, your overall approach to selecting spans and formatting numbers will align well with the task expectations.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment on lines 6 to 9
const total = spanPopulation.reduce(
(sum, n) => sum + Number(n.innerHTML.split(',').join('')),
0,
);

Choose a reason for hiding this comment

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

The reduce-based summation here uses Number(n.innerHTML.split(',').join('')) directly. If any span contains non-numeric text this will produce NaN. Per the requirements you must ensure the string can be converted to a number and filter out invalid entries before summing — e.g. build validNumbers by reading n.textContent.trim(), removing separators, converting with Number(...) and keeping only values where Number.isFinite(value), then reduce over validNumbers.

const averagePopulation = document.querySelector('.average-population');
const totalPopulation = document.querySelector('.total-population');
const total = spanPopulation.reduce(
(sum, n) => sum + Number(n.innerHTML.split(',').join('')),

Choose a reason for hiding this comment

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

Using n.innerHTML can include stray HTML or whitespace. Use n.textContent.trim() to reliably get the span text, then remove thousands separators (for example .replace(/[,\s]/g, '')) before converting to a number.

(sum, n) => sum + Number(n.innerHTML.split(',').join('')),
0,
);
const average = Math.round(total / spanPopulation.length);

Choose a reason for hiding this comment

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

Average is computed as Math.round(total / spanPopulation.length). The requirement is to compute the average based on the count of successfully parsed numeric values (not the original node list). Compute average using the length of the filtered validNumbers array and guard against division by zero (e.g. show 0 or keep 'Calculate it!' if there are no valid numbers).

);
const average = Math.round(total / spanPopulation.length);

function convertNumbers(number) {

Choose a reason for hiding this comment

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

convertNumbers assumes it receives a valid finite number; if you pass NaN or Infinity the output will be wrong. Ensure inputs are validated before calling this function (and round the average before formatting as required).

Comment on lines 28 to 29
averagePopulation.textContent = convertNumbers(average);
totalPopulation.textContent = convertNumbers(total);

Choose a reason for hiding this comment

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

When writing results, you should guard that the elements exist (e.g. if (averagePopulation) { averagePopulation.textContent = ... }). Also ensure you only set textContent (not outerHTML) so the span elements and their classes remain intact and the DOM layout/styles do not change.

Copy link

@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

I am approving your solution as an educational override so you can keep progressing while you practice these concepts.

You did well selecting the .population spans, trimming their text, computing total and average, and creating a separate function to format numbers with thousands separators. The main improvements you should focus on are: (1) build an array of cleaned, parsed numbers using something like .replace(/[, 0\s]/g, ''), Number(...), and Number.isFinite(...) so only valid numbers are included in the total and average [CHECKLIST ITEM #3]; (2) compute the average using the count of valid parsed numbers rather than spanPopulation.length, and guard against division by zero when there are no valid numbers [CHECKLIST ITEM #4]. Also, only call convertNumbers (and update the DOM) when the value you’re formatting is a finite number; otherwise, keep the original text so you don’t show NaN or Infinity [CHECKLIST ITEMS #5/#6]. Finally, the task required changing only src/scripts/main.js, so please revert any changes you made in src/styles/*.scss so the submission fully matches the constraints [CHECKLIST ITEMS #9/#10].

Overall, your structure and intent are solid; tightening validation and reverting style changes will bring this solution fully in line with the requirements.


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

Footnotes

  1. Rate AI review example

const spanPopulation = [...document.querySelectorAll('.population')];
const averagePopulation = document.querySelector('.average-population');
const totalPopulation = document.querySelector('.total-population');
const total = spanPopulation.reduce(

Choose a reason for hiding this comment

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

This reduce sums every span value without validating conversion to a finite number. Checklist #3 requires verifying strings are convertible before using them — instead, build an array of parsed numbers (parse + filter with Number.isFinite) and reduce that array.

const averagePopulation = document.querySelector('.average-population');
const totalPopulation = document.querySelector('.total-population');
const total = spanPopulation.reduce(
(sum, n) => sum + Number(n.textContent.trim().split(',').join('')),

Choose a reason for hiding this comment

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

Cleaning here uses split/join to remove commas. Use a single replace to remove commas and whitespace (including NBSP) before Number(), e.g. .replace(/[,\u00A0\s]/g, ''), then convert and check Number.isFinite(...) (checklist #3).

(sum, n) => sum + Number(n.textContent.trim().split(',').join('')),
0,
);
const average = Math.round(total / spanPopulation.length);

Choose a reason for hiding this comment

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

Average is calculated using spanPopulation.length. The average must be based on the count of valid parsed numbers (not total spans). Also guard against division by zero when there are no valid numbers (checklist #4).

);
const average = Math.round(total / spanPopulation.length);

function convertNumbers(number) {

Choose a reason for hiding this comment

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

convertNumbers assumes it receives a valid finite integer. Ensure you only call it with validated finite numbers (round the average first). Alternatively consider Intl.NumberFormat for thousands separators.

}

if (averagePopulation) {
averagePopulation.textContent = convertNumbers(average);

Choose a reason for hiding this comment

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

When writing results to the DOM you currently overwrite unconditionally. Check that the computed average is finite before calling convertNumbers and setting textContent — otherwise you may replace 'Calculate it!' with NaN or Infinity. The same applies to total (checklist #5/#6).

}

if (totalPopulation) {
totalPopulation.textContent = convertNumbers(total);

Choose a reason for hiding this comment

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

Also check the total before formatting/assigning: only call convertNumbers(total) if Number.isFinite(total) (or compute total from the validated array as suggested).

src: url('../fonts/Roboto-Regular-webfont.woff') format('woff');
font-weight: normal;
font-style: normal;
src: url('../fonts/Roboto-Regular-webfont.woff') format('woff');

Choose a reason for hiding this comment

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

Make sure the font file exists at the given relative path; if the file is missing the @font-face rule will fail and the custom font won't load.

background: #eee;
counter-reset: section;

display: flex;

Choose a reason for hiding this comment

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

The task requires changing only main.js and not styles or HTML (checklist items #9 and #10). This stylesheet appears to have been modified in the submission — please revert any changes to src/styles/main.scss so only main.js is changed.

@@ -18,28 +22,32 @@ h1 {

.list {
margin: 32px 0;

Choose a reason for hiding this comment

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

Further style rules were edited here (e.g. list and spacing). Reverting the entire styles folder (or this file) to the original will ensure the submission follows the task constraint to modify only main.js.

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