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

Introduce a new associateBy() function #223

Merged
merged 3 commits into from Jun 2, 2022

Conversation

Kernald
Copy link
Contributor

@Kernald Kernald commented Oct 25, 2021

Similar to groupBy(), except that it only keeps the latest value corresponding to a given key.

Prior art: Kotlin

@google-cla google-cla bot added the cla: yes label Oct 25, 2021
lib/src/functions.dart Outdated Show resolved Hide resolved
Copy link
Member

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

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

Please add an entry to the changelog, although we should ponder if we want this, too...

@Kernald
Copy link
Contributor Author

Kernald commented Oct 25, 2021

Changelog entry added.

Copy link
Member

@lrhn lrhn left a comment

Choose a reason for hiding this comment

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

I'm not convinced about the need for this function.

What are the already known use-cases that we are aiming to help?

}

return map;
}
Copy link
Member

@lrhn lrhn Oct 25, 2021

Choose a reason for hiding this comment

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

Can be done using a map literal:

  => {for (var element in values) key(element): element};

and I think it should be.

lib/src/functions.dart Outdated Show resolved Hide resolved
test/functions_test.dart Outdated Show resolved Hide resolved
@natebosch
Copy link
Member

@Kernald - are you still interested in this feature?

@Kernald
Copy link
Contributor Author

Kernald commented May 27, 2022

@natebosch Sorry this kind of slipped out of my attention - I still think it would be nice to have. Should I go ahead and update the test as requested, and rename this as lastBy?

@natebosch
Copy link
Member

Yeah if you apply the feedback given so far we can do another round of review and keep this moving forward. Thanks for the contribution!

If you need any help resolving merge conflicts or have any questions I can help.

@Kernald
Copy link
Contributor Author

Kernald commented May 27, 2022

Rebased, and addressed the feedback:

  • renamed to lastBy()
  • Used a String rather than dynamic in the test.

lib/src/functions.dart Outdated Show resolved Hide resolved
Copy link
Member

@lrhn lrhn left a comment

Choose a reason for hiding this comment

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

Very close now. Just need to mention the extension method in the CHANGELOG too.

CHANGELOG.md Show resolved Hide resolved
test/functions_test.dart Outdated Show resolved Hide resolved
lib/src/iterable_extensions.dart Show resolved Hide resolved
lib/src/functions.dart Outdated Show resolved Hide resolved
Similar to groupBy(), except that it only keeps the latest value
corresponding to a given key.

Prior art: [Kotlin](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.sequences/associate-by.html)
@Kernald
Copy link
Contributor Author

Kernald commented Jun 2, 2022

Is there anything left preventing a merge?

@natebosch natebosch merged commit f9b433d into dart-lang:master Jun 2, 2022
@Kernald Kernald deleted the associate-by branch June 3, 2022 07:47
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

4 participants