solution#1970
Conversation
Kit3AWP
commented
Apr 8, 2026
Kit3AWP
commented
- DEMO LINK
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job implementing the core functionality! The list sorting and employee data extraction are working correctly. However, changes are needed for the sortList function. It currently accepts a listElement parameter but incorrectly uses a global list variable to select the items for sorting. This makes the function unreliable and not reusable as intended. Please update the function to use the listElement parameter it receives to find and sort the list items. Addressing this scope issue will make your solution robust and correct.
✨ 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
| 'use strict'; | ||
|
|
||
| // write code here | ||
| const list = document.querySelector('ul'); |
There was a problem hiding this comment.
This global list variable is redundant because employeeList is declared on line 33 for the same purpose. Using global variables can lead to unexpected behavior. It would be better to remove this line and only query the DOM when needed, as is done on line 33.
| const parseSalary = (salaryStr) => Number(salaryStr.replace(/[^\d]/g, '')); | ||
|
|
||
| const sortList = (listElement) => { | ||
| const items = Array.from(list.querySelectorAll('li')); |
There was a problem hiding this comment.
The function sortList receives listElement as a parameter, but here it's using the global list variable. To make the function self-contained and reusable, you should use the listElement parameter to get the list items, for example: const items = Array.from(listElement.querySelectorAll('li'));.
| return salaryB - salaryA; | ||
| }); | ||
|
|
||
| list.append(...items); |
There was a problem hiding this comment.
This line is redundant and incorrectly uses the global list variable. The list is correctly cleared and then populated with the sorted items on lines 18 and 19 using the listElement parameter. This line should be removed to avoid confusion and unnecessary DOM manipulation.
| const getEmployees = (listElement) => { | ||
| const items = [...listElement.querySelectorAll('li')]; | ||
|
|
||
| return Array.from(items).map((li) => ({ |
There was a problem hiding this comment.
A small suggestion for cleaner code: since items is already an array (created with spread syntax ... on line 23), you don't need to wrap it with Array.from(). You can directly call .map() on items.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Good effort on this, you're very close to a complete solution! I am requesting one small change to meet the task requirements. The sortList function correctly accepts a listElement parameter, but it's still using the global list variable to find the list items. To make the function reusable, it must use the parameter it receives. Please update the query inside sortList to use the listElement parameter, like this: const items = listElement.querySelectorAll('li');. Once you make that fix, everything will be perfect!
✨ 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
| const parseSalary = (salaryStr) => Number(salaryStr.replace(/[^\d]/g, '')); | ||
|
|
||
| const sortList = (listElement) => { | ||
| const items = Array.from(list.querySelectorAll('li')); |
There was a problem hiding this comment.
This line is still using the global list variable to query for the list items. The function should use the listElement parameter it receives to make it reusable and independent of global variables, just as you've done in the getEmployees function.
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 submission! You've done a fantastic job addressing the feedback from the last review. The sortList function is now much more robust and reusable because it correctly uses the listElement parameter. The entire solution is clean, well-structured, and perfectly meets all the task requirements. I am approving your work, well done!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
3 participants
