solution

[Demtar23]

DEMO LINK

[mateacademy-ai-mentor]

Review Summary

Great job on implementing the core sorting logic! The list is correctly sorted by salary in descending order.

However, I'm requesting a change because the sortList function doesn't fully meet the task requirements. The function is required to both sort the list items and then append them back to the DOM. Currently, the DOM manipulation happens in the global scope. Please move the logic for clearing the list and appending the sorted items inside the sortList function to encapsulate all related functionality.

Additionally, while the getEmployees function is called correctly, its return value is not used. It's a good practice to use the returned data, for example by logging it to the console, to confirm it's working as expected.

---

✨ 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

This function correctly sorts the array of list items. However, the task description (Step 2) specifies that the sortList function should also be responsible for appending the sorted items back to the DOM. Currently, that logic is handled outside this function on lines 32 and 34.

[mateacademy-ai-mentor]

You are calling the getEmployees function as required, but its return value (the array of employee objects) is not being used. Since this function has no side effects, this line of code has no impact. It's good practice to use the returned data, for example, by logging it to the console: console.log(getEmployees(sortedList));.

[mateacademy-ai-mentor]

Review Summary

Great job on the previous updates! The logic is now correctly placed inside the sortList function.

I am requesting one final change to improve the function's design. The sortList function should be more self-contained by only accepting the list of items as its single parameter. The parent <ul> element, which you are currently passing as a second argument, can be accessed from inside the function using list[0].parentElement.

Refactoring the function to only take one argument will make it more reusable and align it better with the task requirements. You are very close to completing this!

---

✨ 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

To make this function more self-contained and align better with the requirements, it should only accept the list of items as its single parameter. The parent <ul> element, which you are currently passing as newUl, can be accessed from inside the function using list[0].parentElement. This change will make your function more reusable.

[mateacademy-ai-mentor]

Review Summary

Great job on refactoring the sortList function signature! You're very close to a perfect solution.

I am approving this submission as an educational override to help you move forward, but there's one final improvement to make your function truly reusable. The sortList function still depends on the global ul variable, which makes it less portable. A more robust approach is to make the function completely self-contained.

You can achieve this by getting the parent element directly from the list you pass in, like this:

if (!list.length) {
  return list;
}

const parent = list[0].parentElement;
parent.innerHTML = '';
list.forEach(item => parent.appendChild(item));

This small change removes the external dependency and is a great practice for writing professional code. Keep up the excellent work!

---

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

Footnotes

1. Rate AI review example ↩

[mateacademy-ai-mentor]

While this works, to make the function fully self-contained as mentioned in the previous review, it's best to avoid relying on the global ul variable. You can derive the parent element directly from the list that's passed in.

Here's a suggestion:

if (!list.length) {
  return list;
}

const parent = list[0].parentElement;
parent.innerHTML = '';
list.forEach(item => parent.appendChild(item));

This change removes the external dependency, making your function more robust and reusable.