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

CU-f34dk7 remove old mapN and expose new APIs #2292

Merged
merged 7 commits into from Mar 5, 2021

Conversation

nomisRev
Copy link
Member

@nomisRev nomisRev commented Mar 3, 2021

Second, and last, step of the deprecation/removacl top-level mapN for nullable. So now we can expose top-level mapN APIs for Kotlin Std types that don't have a companion object to expose mapN upon.

@nomisRev nomisRev added the 0.13.0 label Mar 3, 2021
@nomisRev nomisRev requested a review from a team March 3, 2021 15:27
@franciscodr
Copy link
Collaborator

@pakoito
Copy link
Member

pakoito commented Mar 3, 2021

This is a hammer to perf, all the nested flatMaps create a new list even when zipping just 2. I'd specialise every implementation.

i.flatMap { ii ->
j.flatMap { jj ->
k.map { kk ->
map(bb, cc, dd, ee, ff, gg, hh, ii, jj, kk)
Copy link
Member

Choose a reason for hiding this comment

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

ayuken

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

Thanks @nomisRev !

mapN(
b,
c,
sequenceOf(Unit),
Copy link
Member

Choose a reason for hiding this comment

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

Does sequence(Unit) allocate new sequences or is it backed by a singleton value? If not we could optimize this.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's probably not backed by a singleton value.

@nomisRev
Copy link
Member Author

nomisRev commented Mar 3, 2021

This is a hammer to perf, all the nested flatMaps create a new list even when zipping just 2. I'd specialise every implementation.

@pakoito For all data types or just List?

@pakoito
Copy link
Member

pakoito commented Mar 3, 2021

Just List for now. Sequence doesn't generate intermediates.

@nomisRev nomisRev merged commit 56eb989 into release/0.13.0 Mar 5, 2021
@nomisRev nomisRev deleted the cu-f34dk7-deprecate-old-mapn branch March 5, 2021 16:07
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.

None yet

6 participants