add task solution#2014
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 because the core required API is not implemented yet, which will block the task from passing. The specification explicitly requires two functions, sortList(list) and getEmployees(list), and for both of them to be called; right now the logic lives at the top level and these functions don’t exist [CHECKLIST ITEM #11, #14, #15]. getEmployees(list) must accept a list element and return an array of { name, position, salary, age } objects; you’ve built this array as a top-level constant instead of returning it from a function [CHECKLIST ITEM #2, #3]. sortList(list) must sort the actual li elements by their data-salary attribute (using your salary conversion helper) and then append those existing lis back to the list, but at the moment you sort plain objects and rebuild innerHTML, which does not meet the DOM-based requirement [CHECKLIST ITEM #1, #11, #12, #13].
To fix this, refactor so that: (1) getEmployees(list) reads list.querySelectorAll('li'), builds and returns the employee array; (2) sortList(list) converts each child li’s dataset.salary with your helper, sorts the nodes, and re-appends them to list in descending salary order; and (3) at the bottom of the file you obtain the ul via document.querySelector('ul'), then call sortList(list) and getEmployees(list). Your salary parsing helper and object shape are good starts, so once they are wired into these two functions and used on the actual DOM elements, you should be in line with the task requirements. Keep going—you’re close, and this refactor will make your solution both cleaner and compliant with the spec.
✨ 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 elementParent = document.querySelector('ul'); |
There was a problem hiding this comment.
| const parseSalary = (salaryStr) => { | ||
| if (!salaryStr) { | ||
| return 0; | ||
| } | ||
|
|
||
| return Number(salaryStr.replace(/[^0-9.]/g, '')) || 0; |
| const arrayListOfEmployees = elemListOfEmployees.map((employee) => { | ||
| const { position, salary, age } = employee.dataset; | ||
| const employeeName = employee.textContent.trim(); | ||
|
|
||
| return { | ||
| name: employeeName, | ||
| position, | ||
| salary, | ||
| age, | ||
| }; | ||
| }); |
| const sortedBySalary = arrayListOfEmployees.sort( | ||
| (a, b) => parseSalary(b.salary) - parseSalary(a.salary), | ||
| ); |
There was a problem hiding this comment.
| const newListHTML = sortedBySalary | ||
| .map((user) => { | ||
| return ` | ||
| <li | ||
| data-position="${user.position}" | ||
| data-salary="${user.salary}" | ||
| data-age="${user.age}" | ||
| > | ||
| ${user.name} | ||
| </li> | ||
| `; | ||
| }) | ||
| .join(''); | ||
|
|
||
| elementParent.innerHTML = newListHTML; |
There was a problem hiding this comment.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because it cleanly meets all of the specified functional requirements. Your sortList(list) function correctly reads the existing li elements from the provided list, parses the data-salary via a helper, sorts them in descending salary order, and re-appends the same DOM nodes rather than rebuilding the list, which aligns with the DOM-focused requirement. Your getEmployees(list) function uses list.querySelectorAll('li') and returns an array of objects with the required shape { name, position, salary, age }, correctly extracting data attributes and text content. Both functions are called after selecting the ul from the document, so the overall flow (find the list, sort it, and then build the employees array) matches the task steps. The helper salary parser is a nice touch for robustness and keeps your main functions clear. Overall, the implementation is concise, readable, and should work well with the automated tests and the given specification.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.