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

Suggestion: List pop function rename #773

Open
Norlock opened this issue Dec 20, 2024 · 7 comments
Open

Suggestion: List pop function rename #773

Norlock opened this issue Dec 20, 2024 · 7 comments

Comments

@Norlock
Copy link

Norlock commented Dec 20, 2024

The list.pop function drops from the beginning of a list, but usually pop means to remove at the end of an array / stack / queue / list and returning that value. Maybe its better to rename this function to something else to prevent confusion.

Current implementation:
https://hexdocs.pm/gleam_stdlib/gleam/list.html#pop

@lpil
Copy link
Member

lpil commented Dec 20, 2024

What might you call these functions?

@Norlock
Copy link
Author

Norlock commented Dec 20, 2024

Maybe just remove?

remove ->

fn remove(list: List(a), fun: fn (Int, a) -> Bool) -> #(Result(a), List(a))

pop ->

fn pop(list: List(a)) -> #(Result(a), List(a)) // returns a list without the last element

filter_index ->

fn filter_index(list: List(a), fun: fn (a, Int) -> Bool) -> List(a)

But I would also remove some of the current remove functions like (drop, drop_while, take, take_while, rest), you can do already so much if you use the filter / filter_index functions like:

list.filter_index(some_list, fn (_item, idx) { idx < 4 }) 

@lpil
Copy link
Member

lpil commented Dec 20, 2024

Lists are not suitable for removing from the end so we would not support that function, and that are not for indexing so we will not add any index related functions.

Do you have examples for the name "remove" being used for this in other languages? If not wouldn't it have the same problem?

@Norlock
Copy link
Author

Norlock commented Dec 20, 2024

Removing the last item is indeed not really needed as a function, however this is what I would expect the function pop to do if it would exist.

https://doc.rust-lang.org/std/vec/struct.Vec.html#method.remove
https://en.cppreference.com/w/cpp/algorithm/remove
https://www.w3schools.com/java/ref_arraylist_remove.asp

Has a remove function which works based on index, but the pop here is actually doing the same, the first result which matches the predicate is removed. Or you can name it remove_first maybe

@lpil
Copy link
Member

lpil commented Dec 20, 2024

None of those functions named remove are the same, so it would be the same problem if we used that name.

You must never use indexes with lists.

@ethanthoma
Copy link
Contributor

I also agree pop is a bad name. It's more like a find_remove type of function since pop functions don't typically have a predicate function

@lpil
Copy link
Member

lpil commented Dec 22, 2024

That makes sense, but find_map_remove seems like too unwieldy a name at that point. It would be good to have something more concise.

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

No branches or pull requests

3 participants