Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes as there are a few critical issues preventing the code from meeting the task requirements. You have the right overall structure, but the implementation needs some adjustments to work correctly.
Here are the key areas to focus on:
- Incorrect Element Selectors: The selectors for fetching data (
li span) and displaying results (.total,.average) are incorrect. Please make sure you are selecting the specific classes mentioned in the requirements:.population,.total-population, and.average-population. - Number Parsing Error: The population numbers in the HTML contain commas, which causes
Number()to returnNaN. You'll need to remove the commas from the string before converting it to a number. - Missing Output Formatting: The final
totalandaveragevalues must be formatted with a thousands separator. ThetoLocaleString()method is a great way to handle this.
Addressing these points will ensure your solution works as expected. You're 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
| // write your code here | ||
| const inside = document.querySelector('.container'); | ||
| const t = inside.querySelector('.list'); | ||
| const list = t.querySelectorAll('li span'); |
There was a problem hiding this comment.
This selector captures all span elements within each list item, including the ones with country names. The task requires getting data only from span elements with the population class. Using a more specific selector will prevent trying to convert non-numeric text to a number.
src/scripts/main.js
Outdated
| const p = Array.from(list); | ||
|
|
||
| const totalSum = p.reduce((acc, curr) => { | ||
| const value = Number(curr.textContent); |
There was a problem hiding this comment.
The population numbers in the HTML (e.g., 1,439,323,776) contain commas. The Number() function cannot parse strings with commas and will return NaN. You'll need to remove these characters before converting the string to a number.
src/scripts/main.js
Outdated
| const l = inside.querySelector('.total'); | ||
| const u = inside.querySelector('.average'); |
There was a problem hiding this comment.
The selectors .total and .average do not match the class names specified in the task description, which are total-population and average-population. The code won't be able to find the correct elements to update.
src/scripts/main.js
Outdated
| l.textContent = totalSum; | ||
| u.textContent = average; |
There was a problem hiding this comment.
Requirement #5 states that the calculated average and total numbers must be formatted with a thousands separator. Currently, they are being displayed as raw numbers. Consider using a method like toLocaleString() to format them correctly.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this solution! All the core requirements have been met. The code correctly calculates and displays the total and average population with the required formatting.
As a final polish to perfectly match the task's preview image, consider rounding the average population to the nearest whole number. The preview shows an integer, but your calculation can produce a decimal. You can achieve this by using Math.round() on your average variable before you format it with toLocaleString().
This is a minor suggestion, and your implementation is functionally correct. I'm approving your submission—great job!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| let average = totalSum / list.length; | ||
|
|
||
| totalSum = totalSum.toLocaleString('en-US'); | ||
| average = average.toLocaleString('en-US'); |
There was a problem hiding this comment.
The task's preview image shows the average population as a whole number (an integer). Your current calculation will result in a number with decimal places. Consider rounding the average value before you format it to a string to match the expected output.
2 participants
No description provided.