Skip to content
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

add max to gleam/list #769

Merged
merged 2 commits into from
Dec 19, 2024
Merged

add max to gleam/list #769

merged 2 commits into from
Dec 19, 2024

Conversation

YilunAllenChen
Copy link
Contributor

Inspired by my advent of code 2024 experiences. I often find myself reinventing list.min and list.max with fold and reduce, so I thought maybe it'd be nice to just promote these.

PR Adds list.min and list.max to gleam/list.

Like the name suggests, it returns the max/min element in the list, using the passed in comparator. These are slightly higher level abstractions than reduce as it takes in comparator functions, which can help with ergonomics at many places.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you! Very useful.

I'm wondering if we really need both, given one is the the other inverted. It's be like having a list.filter and a list.filter_not or such.

Are there any other names we might want to consider?

@YilunAllenChen
Copy link
Contributor Author

Yeah I thought about this, and eventually decided that keeping both is reasonable - here's my train of thoughts

  • Having both feels like slight duplication given they are doing quite similar things, just opposite directions, which kind of goes against Gleam's philosophy to keep things simple and having only one way to do things
  • However I find it hard to find an intuitive name for this unified function - extreme? Terminal? Or just call it max? I have two problems with that - 1. We do have int.max and int.min and so on, and this creates an assymmetry. 2. I think it's be intuitive to reach for both min and max, rather than doing something like list.extreme(int.compare |> function.flip). I think users will also find it counterintuitive to think about "oh I want the max in this list, so I should reach for extreme". That feels like a bigger violation to Gleam's philosophy to stay friendly haha.

Happy to amend it if you have ideas though! Thanks for the prompt response.

@lpil
Copy link
Member

lpil commented Dec 17, 2024

Good reasoning, thank you. Let's move forwards with just max and see how that feels for some time. It's easy to add in future, but it's hard to remove.

@YilunAllenChen
Copy link
Contributor Author

excellent point that it's hard to remove things! Removed min

@YilunAllenChen YilunAllenChen requested a review from lpil December 18, 2024 01:39
@YilunAllenChen YilunAllenChen changed the title add min/max to gleam/list max to gleam/list Dec 18, 2024
@YilunAllenChen YilunAllenChen changed the title max to gleam/list add max to gleam/list Dec 18, 2024
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you!!

@lpil lpil merged commit e610796 into gleam-lang:main Dec 19, 2024
7 checks passed
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