Task completed - #2051
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This code has critical blockers that must be fixed before it can work. The task requires a sortList function that takes a list parameter and a getEmployees function that returns an array of employee objects with {name, position, salary, age} properties - both of these are completely missing. Additionally, line 6 removes all elements before processing, which empties the collection and causes the loop on line 9 to iterate over nothing, so the sorting logic never executes. You need to: (1) Create the sortList function that accepts a list parameter, (2) Create the getEmployees function that extracts employee data from element.dataset.name, element.dataset.position, element.dataset.salary, and element.dataset.age to build the employee objects, and (3) Fix the element removal logic so elements are processed before being cleared. Please review the task requirements and implement these functions as specified.
✨ 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 elements = document.querySelectorAll('[data-salary]'); | ||
| const elementsParent = [...elements][0].parentElement; | ||
|
|
||
| elements.forEach((element) => element.remove()); | ||
|
|
||
| const peopleBySalary = []; | ||
|
|
||
| [...elements].forEach((element) => { | ||
| const salaryValue = element.getAttribute('data-salary'); | ||
|
|
||
| const salary = getTotalSalary(salaryValue); | ||
|
|
||
| peopleBySalary.push([salary, element]); | ||
| }); | ||
|
|
||
| peopleBySalary.sort((value1, value2) => { | ||
| return value2[0] - value1[0]; | ||
| }); | ||
|
|
||
| peopleBySalary.forEach((person, index) => { | ||
| elementsParent.appendChild(peopleBySalary[index][1]); | ||
| }); |
There was a problem hiding this comment.
The task requires a function named sortList that takes a list parameter. This code doesn't define sortList at all - it uses global variables instead. Define the function as specified in the requirements.
| const elements = document.querySelectorAll('[data-salary]'); | ||
| const elementsParent = [...elements][0].parentElement; | ||
|
|
||
| elements.forEach((element) => element.remove()); | ||
|
|
||
| const peopleBySalary = []; | ||
|
|
||
| [...elements].forEach((element) => { | ||
| const salaryValue = element.getAttribute('data-salary'); | ||
|
|
||
| const salary = getTotalSalary(salaryValue); | ||
|
|
||
| peopleBySalary.push([salary, element]); | ||
| }); | ||
|
|
||
| peopleBySalary.sort((value1, value2) => { | ||
| return value2[0] - value1[0]; | ||
| }); | ||
|
|
||
| peopleBySalary.forEach((person, index) => { | ||
| elementsParent.appendChild(peopleBySalary[index][1]); | ||
| }); |
There was a problem hiding this comment.
CRITICAL: The task requires a getEmployees function that returns an array of employee objects with {name, position, salary, age}. This function is completely missing. You must create it to pass the tests.
| const elements = document.querySelectorAll('[data-salary]'); | ||
| const elementsParent = [...elements][0].parentElement; | ||
|
|
||
| elements.forEach((element) => element.remove()); |
There was a problem hiding this comment.
This line removes all elements before processing them. After line 6, elements becomes empty, so the loop on line 9 (using [...elements]) iterates over nothing. This causes the sorting logic to never execute.
|
|
||
| [...elements].forEach((element) => { | ||
| const salaryValue = element.getAttribute('data-salary'); | ||
|
|
||
| const salary = getTotalSalary(salaryValue); | ||
|
|
||
| peopleBySalary.push([salary, element]); | ||
| }); |
There was a problem hiding this comment.
Since elements were removed on line 6, this loop runs on an empty array. The sorting code below never actually processes any data.
| function getTotalSalary(value) { | ||
| let salary = ''; | ||
| const numbers = '0123456789'; | ||
|
|
||
| value.split('').forEach((char) => { | ||
| if (numbers.includes(char)) { | ||
| salary += char; | ||
| } | ||
| }); | ||
|
|
||
| return +salary; |
There was a problem hiding this comment.
The salary parsing logic can be simplified. Instead of manually iterating through characters, use parseInt(value.replace(/\D/g, ''), 10) or Number(value.replace(/\D/g, '')) to extract numbers from the string.
| function getTotalSalary(value) { | ||
| let salary = ''; | ||
| const numbers = '0123456789'; | ||
|
|
||
| value.split('').forEach((char) => { | ||
| if (numbers.includes(char)) { | ||
| salary += char; | ||
| } | ||
| }); | ||
|
|
||
| return +salary; |
There was a problem hiding this comment.
The task requires extracting employee data into an array of objects with {name, position, salary, age} properties from data attributes. Use element.dataset.name, element.dataset.position, etc. to build these objects 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
The getEmployees function has a critical issue where the returned employee objects are missing the required name property. According to the task requirements, the employee schema must include all four properties: name, position, salary, and age. Currently, only position, salary, and age are being extracted. To fix this, add name: element.getAttribute('data-name') to the returned object in the getEmployees function. This missing field is a core requirement that prevents the employee data structure from meeting the task specifications.
✨ 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
| return [...employeeElements].map((element) => { | ||
| return { | ||
| position: element.getAttribute('data-position'), | ||
| salary: getTotalSalary(element.getAttribute('data-salary')), | ||
| age: Number(element.getAttribute('data-age')), | ||
| }; |
There was a problem hiding this comment.
The returned object is missing the name property. The task requires the employee schema to include name, position, salary, and age. Add name: element.getAttribute('data-name') to the returned object.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. Great work implementing both sortList and getEmployees correctly and aligning with the schema by including the name property (e.g., around line 43). The salary parsing and aggregation via getTotalSalary are handled cleanly, and both functions are invoked as required.
-
What you did well:
- Clear data transformation in
getEmployees, including proper type conversion for salaries. - Correct sorting logic in
sortListwith a readable implementation.
- Clear data transformation in
-
Minor improvements (optional):
- Consider adding brief inline comments explaining the intent of
getTotalSalaryand the sort comparator for maintainability.
- Consider adding brief inline comments explaining the intent of
Keep up the solid, clean approach—nicely done!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
DEMO LINK