Skip to content

Conversation

dkuku
Copy link
Contributor

@dkuku dkuku commented Aug 2, 2025

closes #14684
based on the logic of min_max_by

Comment on lines 2148 to 2149
If multiple elements are considered maximal or minimal, the first one
that was found is returned.
Copy link
Member

@josevalim josevalim Aug 3, 2025

Choose a reason for hiding this comment

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

Suggested change
If multiple elements are considered maximal or minimal, the first one
that was found is returned.
By default, the comparison is done with the `<` sorter function,
as the function must not return true for equal elements.

Copy link
Member

Choose a reason for hiding this comment

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

I have updated the text here. The code won't work if it the sorting function returns equal. Can you please apply it accordingly to min_max_by? Then we are good to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@josevalim josevalim merged commit cacdbf6 into elixir-lang:main Aug 3, 2025
13 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Comment on lines -2163 to 2198
"""
@spec min_max(t, (-> empty_result)) :: {element, element} | empty_result
@spec min_max(t, (element, element -> boolean) | module()) ::
{element, element} | empty_result
when empty_result: any
@spec min_max(
t,
(element, element -> boolean) | module(),
(-> empty_result)
) :: {element, element} | empty_result
when empty_result: any
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling Enum.min_max/2 with a 0-arity anonymous function is still valid (like in the last hexdoc example) but it was removed from this function spec, shouldn't it still be documented?

Copy link
Member

Choose a reason for hiding this comment

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

It seems we don't docuyment on the other ones but we should probably be consistent and document on them all.

Copy link
Contributor

Choose a reason for hiding this comment

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

ggVGc pushed a commit to ggVGc/elixir-verbatim that referenced this pull request Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Enum.min_max should take a sorting function

3 participants