Skip to content

Solution#2502

Open
darvexon wants to merge 1 commit intomate-academy:masterfrom
darvexon:develop
Open

Solution#2502
darvexon wants to merge 1 commit intomate-academy:masterfrom
darvexon:develop

Conversation

@darvexon
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because the core requirement of replacing Array.prototype.sort with your custom implementation is not yet fulfilled, which means callers of arr.sort() still use the native behavior instead of your sort2 logic.

The critical issue is in src/arrayMethodSort.js: you define [].__proto__.sort2, but you never assign Array.prototype.sort (or [].__proto__.sort) to delegate to sort2, so the built-in sort is not actually overridden as the task requires; you should add a line after the sort2 function that makes Array.prototype.sort call or reference sort2 so that arr.sort(compareFn) uses your implementation. Once that is wired up, the main functional requirement will be met. As a minor improvement, your basicCompare function has somewhat unconventional mixed-case behavior; consider simplifying it (for example, by consistently comparing String(a) and String(b) in a clearly documented way) so the ordering is predictable for all value types. Another non-blocking point is that your bubble-style adjacent-swap algorithm is acceptable for this exercise, but in real projects you might want to mention or switch to a more efficient sort for large arrays.

Overall, you’re very close: the custom sorter itself looks good for the purposes of this task, you just need to hook it up to Array.prototype.sort and consider refining the comparator and comments to make the behavior clearer.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/arrayMethodSort.js

return this;
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task requires that you implement Array.prototype.sort using the predefined [].__proto__.sort2. This file defines sort2 but never assigns/implements Array.prototype.sort. Add something like Array.prototype.sort = [].__proto__.sort2; or Array.prototype.sort = function(compare) { return this.sort2(compare); }; after the sort2 definition so the built-in sort is replaced by your implementation.

Comment thread src/arrayMethodSort.js
[].__proto__.sort2 = function(compareFunction) {
// write code here
[].__proto__.sort2 = function (compareFunction = basicCompare) {
for (let i = 0; i < this.length - 1; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop implements a simple bubble/adjacent-swap sort by restarting the iteration (i = -1) when a swap occurs. Using bubble sort is acceptable for this task, but consider recommending (or switching to) a more efficient algorithm (e.g., quicksort or mergesort) for larger arrays in real-world code.

Comment thread src/arrayMethodSort.js
Comment on lines +28 to +36
if (aLower !== String(a) && bLower === String(b)) {
return -1;
}

if (bLower !== String(b) && aLower === String(a)) {
return 1;
}

return String(a).localeCompare(String(b));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The basicCompare function tries to prefer values with uppercase letters by using aLower !== String(a) checks. This is a bit unconventional and may behave unexpectedly for non-string items. If the goal is case-insensitive comparison, consider simply using String(a).localeCompare(String(b), undefined, { sensitivity: 'base' }) or document the intended tie-breaking behavior clearly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants