-
Notifications
You must be signed in to change notification settings - Fork 40
Stdlib: Add OneOrBoth and zipLongest on lists and streams
#1003
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
Conversation
jiribenes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is, I think, perfectly fine, but:
🚲, I really like Unison's names for these operations :)
libraries/common/list.effekt
Outdated
| /// ``` | ||
| /// | ||
| /// O(N) | ||
| def zipLongest[A, B](left: List[A], right: List[B]): List[These[A, B]] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bikeshed: Unison calls this align which is a bit nicer, perhaps?
(ditto for streams)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I care more about the name. Honestly, I would not search for this under the name align and don't quite get how this describes the operation well. align is a single word, but I fail to see how this fits the semantics uniquely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funnily enough align is the name used in Haskell :D
https://hackage.haskell.org/package/semialign-1.3.1/docs/Data-Align.html#v:align
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also "just" call it zip and rely on the overload resolution ;)
These and zipLongest on lists and streamsOneOrBoth and zipLongest on lists and streams
jiribenes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still prefer a different name than zipLongest, but I'm fine with the contents, so feel free to merge this if you want to, @marzipankaiser
No description provided.