Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a core requirement of the task is not yet satisfied: Array.prototype.sort is not implemented using your sort2 method, so the built-in sort still does not use your custom logic. In src/arrayMethodSort.js, after you define sort2 on the array prototype, you should either assign it directly (e.g., Array.prototype.sort = Array.prototype.sort2;) or define sort as a wrapper that calls this.sort2(compareFunction) so that all arrays use your implementation when sort is called. Inside the sort2 implementation, the default comparator currently returns 1 when a > b and -1 otherwise, which incorrectly returns -1 even when a and b are equal, violating the standard sort contract. Please update the default comparator to explicitly handle equality, for example:
(a, b) => {
const A = String(a);
const B = String(b);
if (A > B) return 1;
if (A < B) return -1;
return 0;
}so that equal values return 0. Your bubble sort implementation, destructuring swap, and prototype usage look good for this exercise; once you wire sort to sort2 and fix the equality case in the comparator, the solution will meet the main functional requirements. Keep going—these are focused, fixable changes and you’re very close to a complete solution.
✨ 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
| // write code here | ||
| [].__proto__.sort2 = function (compareFunction) { | ||
| const callback = | ||
| compareFunction || ((a, b) => (String(a) > String(b) ? 1 : -1)); |
There was a problem hiding this comment.
The default comparator used here returns 1 when a > b and -1 otherwise, which also returns -1 when a and b are equal. Per the usual sort contract, the comparator should return 0 when values are equal. Consider adjusting to return 0 for equality, for example by checking < and > explicitly and returning 0 when neither is true.
| } | ||
| } | ||
|
|
||
| return this; |
There was a problem hiding this comment.
The task requires implementing Array.prototype.sort using the predefined [].__proto__.sort2. This file only defines sort2 but never assigns/implements sort to use it. Add an assignment after defining sort2, for example: [].__proto__.sort = [].__proto__.sort2; (or Array.prototype.sort = Array.prototype.sort2;) so Array.prototype.sort uses your implementation.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because your custom sort2 implementation is never actually connected to Array.prototype.sort, so calling [].sort() will still use the native implementation instead of your bubble sort. In src/arrayMethodSort.js, inside applyCustomSort, you correctly define [].__proto__.sort2, but after that point you must reassign the built-in sort, for example:
Array.prototype.sort = Array.prototype.sort2;
// or
Array.prototype.sort = function (compareFunction) {
return this.sort2(compareFunction);
};Without this wiring, a core requirement of the task—making all arrays use your custom logic when sort is called—is not satisfied, which is a functional blocker. The bubble sort itself and the default comparator (including the 0 case for equal values) look good and are fine for this exercise. Once you connect Array.prototype.sort to sort2 inside applyCustomSort, your solution will meet the task’s main functional requirement, and the rest of your implementation won’t need major changes. Keep up the good work; you’re very close to a fully correct solution.
✨ 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 this; |
There was a problem hiding this comment.
After you define sort2, you need to make the built-in Array.prototype.sort use it. Add the wiring inside applyCustomSort (after the sort2 assignment). Example fixes:
Array.prototype.sort = Array.prototype.sort2;
orArray.prototype.sort = function(compareFunction) { return this.sort2(compareFunction); };
Without this, callingarr.sort()will not use your custom logic and the task requirement is not satisfied.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because it correctly defines applyCustomSort so that Array.prototype.sort delegates to your custom sort2, and the sort operates in-place with the expected default lexicographical behavior. This is an educational approval override to let you move forward while you refine a couple of details in your implementation. You did well by wiring sort2 onto the array prototype, reassigning Array.prototype.sort to call it, and implementing a clear bubble-sort loop that returns the mutated array. A key issue to fix is comparator validation in src/arrayMethodSort.js (around the lines where you handle compareFunction inside sort2): the native sort should throw a TypeError when a non-function comparator is passed, so you should explicitly check typeof compareFunction and throw if it is provided but not a function. For larger datasets, the current bubble sort (nested loops in sort2) is inefficient (O(n^2)), so as a next improvement you could either add an early-exit flag when no swaps occur in a pass or replace it with a more efficient algorithm such as merge sort or quicksort. These issues don’t block the core behavior for this exercise, but addressing them will make your custom sort more robust and closer to the native implementation. Overall, nice work meeting the main requirements; refining the comparator checks and performance will strengthen your understanding of how array sorting works under the hood.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.