Skip to content

Conversation

@RossZhang-cpu
Copy link
Contributor

@RossZhang-cpu RossZhang-cpu commented Jul 20, 2023

New Recipe SorteSetStreamCollectToLinkHashSet to address issue #70

What's changed?

This pull request adds a new recipe called SorteSetStreamCollectToLinkHashSet that converts streams of sorted sets to LinkHashSet for collection instead of HashSet.

What's your motivation?

The motivation is to address issue #70

Anyone you would like to review specifically?

@timtebeek @yeikel, thank you in advance.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ auto-formatter on affected files
  • I've updated the documentation (if applicable)

@RossZhang-cpu RossZhang-cpu reopened this Jul 23, 2023
@RossZhang-cpu RossZhang-cpu changed the title SorteSetStreamCollectToLinkHashSet recipe, issue New Recipe of Sorte Set Stream should be collect to LinkHashSet Jul 23, 2023
@timtebeek timtebeek changed the title New Recipe of Sorte Set Stream should be collect to LinkHashSet Explicitly sorted streams should be collected to a LinkedHashSet to preserve order Jul 23, 2023
Move visitor into recipe, as it's not reused
Add additional tests & apply formatter
Collapse conditionals into single if statement
Remove move imports
@timtebeek timtebeek marked this pull request as ready for review July 23, 2023 19:25
@timtebeek
Copy link
Member

Thanks a lot for implementing this @RossZhang-cpu ! I like how you figured out all the steps to adding a method matcher, and finding both the select and argument to that method to determine when to apply the changes. Very nice first contribution here if I'm not mistaken.

I've gone ahead and gave it a quick polish in 802928a with more details in the commit message, mostly to shorten it such that there's less to maintain. Thanks again & hope you'll consider contributing more! :)

@timtebeek
Copy link
Member

Failures unrelated; and only in Kotlin which is actively worked on by @traceyyoshima today. Something to figure out on the main branch.

@RossZhang-cpu
Copy link
Contributor Author

RossZhang-cpu commented Jul 24, 2023

@timtebeek Thank you very much! I am very excited to see my first pr was merged in a short time. This has been an invaluable opporturnity for me to learn something new and improve through this project. And it couldn't be done without your and @yeikel's kind help and guidence.

Your willingness to take the time to help new contributors like me is greatly appreciated. Thank you again and I will definitely contribute more:)

@timtebeek
Copy link
Member

Glad to hear! Anything tagged "good first issue" is typically good to pick up; feel free to tag me in any issue that you're considering such that I can help see if there's any notes for the implementation. I look forward to what you'll do next!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Recipe : sorted set stream should be collected to a LinkedHashSet

3 participants