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

groupByWhile seems unintuitive #135

Closed
rapind opened this issue Feb 7, 2020 · 2 comments
Closed

groupByWhile seems unintuitive #135

rapind opened this issue Feb 7, 2020 · 2 comments

Comments

@rapind
Copy link

rapind commented Feb 7, 2020

I was expecting to pass a list and a grouping function and to get a list of lists back. E.g.
groupWhile (\a b -> a.id == b.id) records -> List (List Record)

Instead we get a list of tuples with the item passed as the first and not included in the list of records. So I have to do a map to remove unwrap the tuple and add the Tuple.first into the list. List.map (\g -> ( optionalLabelFunction (Tuple.first g), Tuple.first g :: Tuple.second g )) groups

When grouping by even simple types like a list of string or ints it's awkward that the item being matched on is removed from the list and made to be the first item in the tuple. I can see why you opted to return a tuple to show what was grouped on, but not why it's removed from the group itself.

I think it would be valuable for records and cover all use cases if you could pass a function for the group label (first in tuple) and still include the record in the list, or simply make it a List of Lists.

(I don't mean to sound unappreciative. List.Extra has been a boon for me!)

@pzp1997
Copy link
Contributor

pzp1997 commented Feb 9, 2020

The reason each group is a ( a, List a ) rather than a List a is so that the fact that a group cannot be empty is encoded in the return type. ( a, List a ) is a fairly common pattern in Elm for expressing a non-empty list. In order to convert from a non-empty list to a normal list, you can define a helper function in your code like

fromNonempty : ( a, List a ) -> List a
fromNonempty ( x, xs ) =
    x :: xs

groupWhile used to return a List (List a) up until July 2018. To see the full discussion that led to the decision to change it to how it is now, see #49 and #105.

@rapind
Copy link
Author

rapind commented Feb 9, 2020

Understood and thanks for the comprehensive background behind the decision! While I would definitely prefer List (List a) as I find that more intuitive / obvious, I can also see that there was an intelligent (and interesting) discussion about it.

As an aside, the pattern matching helper you provided is definitely prettier than what I was doing. Cheers!

@rapind rapind closed this as completed Feb 9, 2020
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

2 participants