Skip to content

Improve documentation/error on Array.sort_custom() function comparison #104944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Apr 2, 2025

@Calinou Calinou requested review from a team as code owners April 2, 2025 22:47
@Calinou Calinou added enhancement cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release labels Apr 2, 2025
@Calinou Calinou added this to the 4.5 milestone Apr 2, 2025
@Calinou Calinou force-pushed the doc-array-sort-function-equal branch from e0f4624 to 8bf20eb Compare April 2, 2025 22:48
Comment on lines 291 to +292
Similar to [method sort_custom], [param func] is called as many times as necessary, receiving one array element and [param value] as arguments. The function should return [code]true[/code] if the array element should be [i]behind[/i] [param value], otherwise it should return [code]false[/code].
If the order of the two elements does not matter (e.g. they are equal), it should return [code]false[/code]. This means that for comparisons, [code]<[/code] or [code]>[/code] should always be used instead of [code]<=[/code] or [code]>=[/code]. If this rule is not followed, an error is printed as sorting will be broken.
Copy link
Member

Choose a reason for hiding this comment

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

I personally feel like this is an overcorrection. The current PR adds a tip smacked right in the middle of the details, which is a bit noisy to the reader.

Suggested change
Similar to [method sort_custom], [param func] is called as many times as necessary, receiving one array element and [param value] as arguments. The function should return [code]true[/code] if the array element should be [i]behind[/i] [param value], otherwise it should return [code]false[/code].
If the order of the two elements does not matter (e.g. they are equal), it should return [code]false[/code]. This means that for comparisons, [code]<[/code] or [code]>[/code] should always be used instead of [code]<=[/code] or [code]>=[/code]. If this rule is not followed, an error is printed as sorting will be broken.
Similar to [method sort_custom], [param func] is called as many times as necessary, receiving one array element and [param value] as arguments. The function should return [code]true[/code] if the array element should be [i]behind[/i] [param value], otherwise it should return [code]false[/code]. If the two elements are considered equivalent, it should also return [code]false[/code].

If absolutely necessary, the additional explanation should be a note below. Although, it is my opinion that, while correct, it may be an oversimplification to address the error.

Comment also applies to sort_custom's description below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Array.sort_custom callable requirements more explicit for the a == b case
2 participants