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

Code contribution for JDK8 collectors support #2591

Closed
cowwoc opened this issue Oct 6, 2016 · 13 comments
Closed

Code contribution for JDK8 collectors support #2591

cowwoc opened this issue Oct 6, 2016 · 13 comments

Comments

@cowwoc
Copy link

cowwoc commented Oct 6, 2016

Hi,

I read that you plan on adding JDK8 support in version 21. I want to bring your attention to https://bitbucket.org/cowwoc/guava-jdk8/ which I authored a few months ago.

Feel free to borrow design ideas and/or code sniplets for your upcoming release.

@kevinb9n
Copy link
Contributor

kevinb9n commented Oct 6, 2016

Thanks for this tip. I'm sorry to say that we've done a large amount of
work on this that you just haven't gotten the chance to see yet. :-( We're
trying to get 20.0 out the door and then dump this code out to you. When
that happens, we'd appreciate your suggestions for making it better.

On Thu, Oct 6, 2016 at 10:44 AM, Gili Tzabari [email protected]
wrote:

Hi,

I read that you plan on adding JDK8 support in version 21. I want to bring
your attention to https://bitbucket.org/cowwoc/guava-jdk8/ which I
authored a few months ago.

Feel free to borrow design ideas and/or code sniplets for your upcoming
release.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2591, or mute the thread
https:/notifications/unsubscribe-auth/AA5Cl-C8rKtJFOFSXwInWpz3Sht2GQeJks5qxTOKgaJpZM4KQOOt
.

Kevin Bourrillion | Java Librarian | Google, Inc. | [email protected]

cgdecker added a commit that referenced this issue Nov 3, 2016
(Note: This will break the public Guava GWT tests. We'll come back to fix them.)

Fixes #2585, #2479, #2029, and probably various other issues :)

Also relevant: #2591

-------------
Created by MOE: https:/google/moe
MOE_MIGRATED_REVID=138100073
@cpovirk cpovirk closed this as completed Nov 3, 2016
@cpovirk
Copy link
Member

cpovirk commented Nov 3, 2016

Err, didn't mean to close this one, sorry.

@cpovirk cpovirk reopened this Nov 3, 2016
@robertmassaioli
Copy link

I just wrote this up really quickly. I assume that you have an even better, more efficient version, in the works: https://medium.com/@robertmassaioli/an-immutablemultimapcollector-for-guava-3f141f9040f#.m18fnmgwo

@cpovirk
Copy link
Member

cpovirk commented Nov 4, 2016

We've mirrored out our implementations (along with other Java 8 changes). Let us know if you have any particular suggestions.

@jbduncan
Copy link
Contributor

jbduncan commented Nov 5, 2016

I'd be interested in seeing a map stream class in a similar vein to OpenGamma Strata's MapStream or StreamEx's EntryStream.

I don't have any uses for such a class yet, but I imagine it would be useful if one needs to do something where they'd either need to use deep-nested chaining of the functional methods in common.collect.Maps or map.entrySet().stream(), both of which in my opinion are a bit clunky.

@jbduncan
Copy link
Contributor

jbduncan commented Nov 7, 2016

@cpovirk I realise that your invitation for particular suggestions was probably aimed at @cowwoc, so would you prefer that I raise my MapStream suggestion above as a new issue?

@cpovirk
Copy link
Member

cpovirk commented Nov 7, 2016

Now that you mention it, a new issue is probably the way to go for any suggestions, given how broad this feature request currently is. I'll close it, and yes, please do open a new issue. Everyone else, feel free to do the same.

@cpovirk cpovirk closed this as completed Nov 7, 2016
@cowwoc
Copy link
Author

cowwoc commented Nov 7, 2016

Please let me know if you create a separate issue. I will add my comments there. Thank you.

@jbduncan
Copy link
Contributor

@cowwoc If there are any noteworthy things in your library that you'd like Guava to consider (or you still have any other comments you'd like to share), I suggest creating a new issue rather than waiting for @cpovirk to create one that may never come. :)

@cowwoc
Copy link
Author

cowwoc commented Nov 12, 2016

@jbduncan The main feature I was trying to propose by opening this issue was offering a builder design pattern for Collectors in order to improve readability. I think this is one of the main features that sets my API apart from others.

@cowwoc
Copy link
Author

cowwoc commented Nov 12, 2016

Example:

ImmutableSetMultimap<Integer, String> sortedMultimap = Stream.of(10, 20, 40, 30, 15).
            collect(GuavaCollectors.multimapFrom(Integer.class).
                               map(i -> i / 10, i -> Integer.toString(i)).
                               asImmutableSetMultimap().
                               sortKeys((a, b) -> b.compareTo(a)).
                               sortValues((a, b) -> b.compareTo(a)).
                               build());

My API still offers the static factory methods that others have implemented but for improved readability or advanced use-cases, I recommend the builder pattern.

Also, notice the way it's implemented users are guided forward by a multi-step DSL. At each step of the DSL, code-complete will offer a different set of methods until a terminal method is invoked.

@jbduncan
Copy link
Contributor

jbduncan commented Nov 12, 2016

Ah I see, looks rather interesting to me. I especially like the DSL-y, aids-with-code-completion idea!

However I still think it's worth creating a new issue with this builder-pattern-based collector idea as the topic, as this issue is currently closed and the Guava team may never visit it again.

@cowwoc
Copy link
Author

cowwoc commented Nov 12, 2016

@jbduncan Done: #2640

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

5 participants