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 met: Array.prototype.sort is not overridden to use your custom sort2 implementation, so any tests or callers that rely on the standard sort method will not see your custom behavior. The implementation of sort2 itself in src/arrayMethodSort.js is generally good and follows the constraints, but right now it only lives as [].__proto__.sort2, which does not satisfy the requirement to “implement your own Array.prototype.sort using that predefined sort2.”
Critical issues to fix:
- After defining
sort2on the array prototype (e.g.[].__proto__.sort2 = function(compare) { ... }), explicitly wire it toArray.prototype.sort. For example, near the end ofapplyCustomSort()(or wherever you installsort2), add something like:or call the same logic withArray.prototype.sort = function(compareFunction) { return this.sort2(compareFunction); };
.call(this); the key is thatArray.prototype.sortdelegates to the already-definedsort2. - Make sure
applyCustomSort()is actually invoked in the environment where tests run (or exported and called by tests) so thatsort2and the overriddensortare installed before any sorting is performed.
Minor improvement (not blocking, but recommended): before using the comparator, check its type and throw a TypeError if compareFunction is non-undefined and not a function, which is closer to the native behavior. Overall, you’re very close—your custom sorting logic looks solid; you just need to connect it to Array.prototype.sort and ensure it’s initialized so the solution fully meets the task requirements.
✨ 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
src/arrayMethodSort.js
Outdated
| function applyCustomSort() { | ||
| [].__proto__.sort2 = function(compareFunction) { | ||
| // write code here | ||
| [].__proto__.sort2 = function (compareFunction) { |
There was a problem hiding this comment.
You define the sort2 method on [].__proto__ here, but the task requires implementing Array.prototype.sort using that predefined sort2. Currently there is no assignment that overrides Array.prototype.sort, so tests expecting a custom sort will fail.
| this[j + 1] = current; | ||
| } | ||
|
|
||
| return this; |
There was a problem hiding this comment.
After the sort2 implementation you should assign Array.prototype.sort to use it, e.g. Array.prototype.sort = function(compare) { return this.sort2(compare); };. Also ensure applyCustomSort() is called (or tests will call it) so the methods are actually installed.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because your custom sort implementation is never actually activated at runtime, which is a critical blocker for the task requirements. In src/arrayMethodSort.js, you correctly define sort2 and wire Array.prototype.sort to delegate to it inside applyCustomSort, but since applyCustomSort() is only exported and never called, the prototypes remain unchanged and tests expecting the overridden Array.prototype.sort will fail. To fix this, either call applyCustomSort() at the bottom of arrayMethodSort.js so it runs on module load, or ensure your test/entry code explicitly imports and invokes applyCustomSort() before any sorting occurs. In src/index.html, you currently just include arrayMethodSort.js without calling applyCustomSort(), and because the file uses CommonJS (module.exports), including it directly in the browser can also cause a ReferenceError for module. A simple fix for the HTML side is to either bundle the code properly (so the module runs and installs the sort on load) or add a small inline script after the <script src="arrayMethodSort.js"> tag that calls applyCustomSort() from a global, if you choose to expose it that way. Aside from this activation issue, the core logic (comparator validation, default comparator, bubble sort, destructuring swap, and delegation from sort to sort2) looks solid, so once the installer is invoked correctly your solution should meet the requirements. Keep going—you’re very close; just ensure the custom sorting behavior is actually installed before any arrays are sorted.
✨ 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
| <title>arrayMethodSort</title> | ||
| </head> | ||
| <body> | ||
| <script src="./arrayMethodSort.js"></script> |
There was a problem hiding this comment.
The page only loads arrayMethodSort.js but does not invoke the exported applyCustomSort() installer. As a result your custom sort2/sort will not be installed — call applyCustomSort() (or modify the module to call it on load) so the prototype changes take effect before any sorting is done.
src/arrayMethodSort.js
Outdated
| return this; | ||
| }; | ||
|
|
||
| [].__proto__.sort = function (compareFunction) { |
There was a problem hiding this comment.
You wired Array.prototype.sort to call sort2 here — good. However, applyCustomSort is only exported and never invoked, so the custom methods are not actually installed at runtime. Call applyCustomSort() during module initialization (for example, just before module.exports) or ensure the test setup calls this exported function so the prototype changes take effect.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you learn, but there are a couple of important issues to fix next.
You did a solid job implementing the custom sort logic itself: the comparator validation, default comparator, and the O(n²) insertion-style algorithm are all clear and correctly attached as non-enumerable prototype methods. The main functional gap is in src/arrayMethodSort.js: Array.prototype.sort currently calls your internal customSort directly, but the requirement is that sort must delegate to the predefined [].__proto__.sort2 (for example, return this.sort2(compareFunction);), so that sort is explicitly based on sort2. Also in the same file, applyCustomSort() is only exported and never invoked, which means sort2 and the overridden sort are never actually installed on Array.prototype at runtime—add a call like applyCustomSort(); after its definition or ensure your entry code imports and calls it before any sorting happens. Finally, in src/index.html, you load arrayMethodSort.js with a plain <script> tag even though the file uses module.exports, which can cause ReferenceError: module is not defined in the browser; either bundle the file for the browser, expose applyCustomSort on window and call it, or convert it to an ES module and import/execute the installer.
Overall, your core sorting logic is well thought out; focusing on how and when the installer runs, and aligning sort to delegate to sort2, will bring your solution fully in line with the task requirements. Keep going—you’re very close to a clean, fully compliant implementation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| <title>arrayMethodSort</title> | ||
| </head> | ||
| <body> | ||
| <script src="./arrayMethodSort.js"></script> |
There was a problem hiding this comment.
This line loads arrayMethodSort.js directly in the browser. The implementation file uses CommonJS (module.exports) and exports an installer function instead of running on load, which can cause ReferenceError: module is not defined and will not install the custom sort. Consider bundling the file for the browser, exposing and calling applyCustomSort() from a global, or converting the implementation to an ES module and importing it so the installer runs.
|
|
||
| Object.defineProperty(proto, 'sort', { | ||
| value: function (compareFunction) { | ||
| return customSort.call(this, compareFunction); |
There was a problem hiding this comment.
Requirement: the task requires sort to be implemented using the predefined [].__proto__.sort2. Here sort calls customSort directly — change this so sort delegates to sort2. For example: return proto.sort2.call(this, compareFunction); or return this.sort2(compareFunction); so the implementation is based on the predefined sort2 method (matches the task spec).
| }); | ||
| } | ||
|
|
||
| module.exports = applyCustomSort; |
There was a problem hiding this comment.
Critical: applyCustomSort is only exported and never invoked. Because the installer isn't called, sort2 and the overridden sort are not installed on Array.prototype at runtime. Call applyCustomSort() (for example add applyCustomSort(); after this definition) or ensure your entry/test code imports and invokes it before any sorting occurs. Also be aware that module.exports = ... won't work when the file is included directly in a browser (see src/index.html) — either bundle the module or expose+call the installer from a browser-compatible script.
No description provided.