Skip to content

first solution#2503

Open
Slava-v-Ukraini wants to merge 3 commits intomate-academy:masterfrom
Slava-v-Ukraini:develop
Open

first solution#2503
Slava-v-Ukraini wants to merge 3 commits intomate-academy:masterfrom
Slava-v-Ukraini:develop

Conversation

@Slava-v-Ukraini
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 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/arrayMethodSort.js Outdated
// write code here
[].__proto__.sort2 = function (compareFunction) {
const callback =
compareFunction || ((a, b) => (String(a) > String(b) ? 1 : -1));
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 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.

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

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

  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.

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;
    or
  • Array.prototype.sort = function(compareFunction) { return this.sort2(compareFunction); };
    Without this, calling arr.sort() will not use your custom logic and the task requirement is not satisfied.

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

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

Footnotes

  1. Rate AI review example

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