add first task solution by shyshenok81-hub · Pull Request #2189 · mate-academy/js_get_data_DOM

shyshenok81-hub · 2026-02-22T14:52:25Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still a couple of critical behavior issues that can break the required functionality. First, the requirement explicitly says you must ensure the string can be converted to a number before converting it, but your current logic calls Number(clean) and only then checks isNaN, which violates that requirement and can lead to confusing handling of invalid inputs; instead, validate clean (e.g., with /^\d+$/ or similar) and only convert when it passes. Second, if no valid population values are found, nums.length will be 0, and total / nums.length will yield Infinity or NaN, which then gets written into the DOM; you need a guard for the empty array case (for example, skip updating the DOM or explicitly set total/average to 0).

In addition, your average formatting currently uses maximumFractionDigits: 0, which implicitly rounds the number; you should either apply an explicit rounding strategy like Math.round, Math.floor, or Math.ceil before formatting, or clearly choose and implement the intended behavior. Once you validate strings before conversion, handle the zero-length case safely, and make the rounding behavior explicit, the rest of your solution (selection of span.population, parsing, totaling, averaging, and DOM updates) looks solid. You’re very close—fixing these points will make your implementation both robust and fully aligned with the task requirements.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-02-22T14:56:25Z

src/scripts/main.js

+const listNum = list.map((el) => {
+  const text = el.textContent.trim();
+  const clean = text.replace(/,/g, '');
+  const number = Number(clean);


The requirement states you must ensure the string "can be converted to a number" before converting it. Here you call Number(clean) and only after that check isNaN. Consider validating clean (for example with /^\d+$/ or similar) before converting to make the intent explicit and fully match the requirement.

mateacademy-ai-mentor · 2026-02-22T14:56:25Z

src/scripts/main.js

+
+const total = nums.reduce((sum, num) => sum + num, 0);
+
+const avarage = total / nums.length;


If nums.length is 0 (no valid population values), total / nums.length will produce Infinity or NaN. Add a guard for an empty array of numbers (e.g., skip calculation or set total/average to 0) to avoid writing invalid values into the DOM.

mateacademy-ai-mentor · 2026-02-22T14:56:25Z

src/scripts/main.js

+
+const formattedTotal = total.toLocaleString('en-US');
+
+const formattedAverage = avarage.toLocaleString('en-US', {


You format the average with maximumFractionDigits: 0, which rounds the average to an integer. If an explicit rounding strategy is required (round, floor, ceil), apply it before formatting or document the chosen strategy; otherwise this implicit rounding may not match expectations.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still a couple of critical issues that can break functionality and violate explicit checklist requirements. First, when no valid population values are found (e.g., nums.length === 0), total and avarage remain undefined, so calling total.toLocaleString(...) and avarage.toLocaleString(...) will throw a runtime error; you need to either guard the formatting/DOM update behind a check that nums.length > 0 or initialize total and average to safe defaults like 0 before formatting [CHECKLIST ITEM #22]. Second, the requirements say to only replace the exact placeholder text Calculate it!, but your code currently always overwrites the .total-population and .average-population span contents; you should check something like if (el.textContent.trim() === 'Calculate it!') before assigning [CHECKLIST ITEM #19]. These two issues are considered blockers because they can cause errors in valid scenarios and do not follow the spec about placeholder replacement. As a minor (non-blocking) improvement, please also rename avarage to average to improve clarity and avoid confusion. Once you handle the empty-data case safely and conditionally update the placeholders only when they equal Calculate it!, your solution will fully meet the task requirements.

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-02-22T17:22:04Z

src/scripts/main.js

+const formattedTotal = total.toLocaleString('en-US');
+
+const formattedAverage = avarage.toLocaleString('en-US');


If nums.length is 0 then total and avarage remain undefined; calling total.toLocaleString(...) / avarage.toLocaleString(...) will throw. Wrap formatting/DOM updates in a branch that runs only when you have valid numbers or assign sensible defaults (e.g. 0) before formatting. This fixes the empty-data case required by checklist item #22.

mateacademy-ai-mentor · 2026-02-22T17:22:04Z

src/scripts/main.js

+document.querySelector('.total-population').textContent = formattedTotal;
+
+document.querySelector('.average-population').textContent = formattedAverage;


The task says to "Replace Calculate it!" — you should only replace the span text when it currently equals exactly Calculate it!. Right now you always overwrite the .total-population and .average-population contents. Check el.textContent.trim() === 'Calculate it!' (or similar) before assigning to meet checklist item #19.

mateacademy-ai-mentor · 2026-02-22T17:22:04Z

src/scripts/main.js

+const nums = listNum.filter((val) => val !== null);
+
+let total;
+let avarage;


avarage is misspelled — rename to average to avoid confusion. This is not strictly required by the checklist but improves clarity and maintainability.

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational override so you can keep progressing while you refine a couple of details.

You did a solid job collecting all span.population elements, validating numeric content, and computing + formatting the total and average with toLocaleString('en-US'), which matches the requirements. The remaining blocking issues are small but important: in src/scripts/main.js, the variable avarage should be renamed to average everywhere it’s used to improve clarity and align with the naming expectations. Also, for both the total and average spans (including the empty-data branch), you currently overwrite their text unconditionally; per the spec, you should only replace the content when textContent.trim() === 'Calculate it!' so that any non-placeholder custom text is never lost [CHECKLIST ITEM #19].

To fix this, wrap each assignment to the .total-population and .average-population spans in an if that checks for the exact placeholder string before setting the new value or the '0' fallback. Overall, your logic and structure are on the right track—tightening these last conditions and the variable name will bring your solution fully in line with the requirements.

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-22T18:44:39Z

src/scripts/main.js

+if (nums.length > 0) {
+  const total = nums.reduce((sum, num) => sum + num, 0);
+
+  const avarage = Math.round(total / nums.length);


Rename the variable avarage to average to improve clarity and follow the checklist note. This avoids confusion and matches the expected naming in requirements.

mateacademy-ai-mentor · 2026-02-22T18:44:39Z

src/scripts/main.js

+  const avgEl = document.querySelector('.average-population');
+
+  if (totalEl) {
+    totalEl.textContent = formattedTotal;


You assign the formatted total unconditionally here. Per the task requirements you must only replace the placeholder text exactly equal to Calculate it!. Wrap the assignment with a check such as if (totalEl.textContent.trim() === 'Calculate it!') before setting the content.

mateacademy-ai-mentor · 2026-02-22T18:44:39Z

src/scripts/main.js

+  }
+
+  if (avgEl) {
+    avgEl.textContent = formattedAverage;


Same issue for average: do not overwrite unconditionally. Check the current text exactly equals Calculate it! (e.g., if (avgEl.textContent.trim() === 'Calculate it!')) before assigning the formatted average.

mateacademy-ai-mentor · 2026-02-22T18:44:39Z

src/scripts/main.js

+  const avgEl = document.querySelector('.average-population');
+
+  if (totalEl) {
+    totalEl.textContent = '0';


In the empty-data branch you set the .total-population span to '0' always. The spec requires replacing only the exact placeholder Calculate it!. Apply the same exact-text guard here before assigning the fallback value.

mateacademy-ai-mentor · 2026-02-22T18:44:39Z

src/scripts/main.js

+  }
+
+  if (avgEl) {
+    avgEl.textContent = '0';


Similarly, only set the .average-population span to '0' when it currently contains the exact placeholder Calculate it!. Add a guard like if (avgEl.textContent.trim() === 'Calculate it!') before assigning.

add first task solution

6734797

mateacademy-ai-mentor suggested changes Feb 22, 2026

View reviewed changes

add second solution

15c617a

shyshenok81-hub requested a review from mateacademy-ai-mentor February 22, 2026 17:17

mateacademy-ai-mentor suggested changes Feb 22, 2026

View reviewed changes

add third solution

b4d4a94

shyshenok81-hub requested a review from mateacademy-ai-mentor February 22, 2026 18:41

mateacademy-ai-mentor approved these changes Feb 22, 2026

View reviewed changes


		const total = nums.reduce((sum, num) => sum + num, 0);

		const avarage = total / nums.length;


		const formattedTotal = total.toLocaleString('en-US');

		const formattedAverage = avarage.toLocaleString('en-US', {

		document.querySelector('.total-population').textContent = formattedTotal;

		document.querySelector('.average-population').textContent = formattedAverage;

Conversation

shyshenok81-hub commented Feb 22, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants