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

Factory method to create a FluentIterable from an array #1070

Closed
gissuebot opened this issue Oct 31, 2014 · 16 comments
Closed

Factory method to create a FluentIterable from an array #1070

gissuebot opened this issue Oct 31, 2014 · 16 comments
Assignees
Milestone

Comments

@gissuebot
Copy link

Original issue created by drothmaler on 2012-07-17 at 03:04 PM


I think it would be nice to be able to create a FluentIterable from an array directly, with a factory method like this:
FluentIterable.from(E... elements) or FluentIterable.from(E[] elements)

This could simply call Arrays.asList(elements) internally, as I am doing now explicitly on the outside.
It is very usefull for working with legacy code or other APIs returning arrays.
Also it feels to be consistent, as there is a "toArray" method; so it would be a logical consequence to provide a from(Array) factory method also.

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2012-07-24 at 05:29 PM


Again, will put on the API review this week.


Status: Research

@gissuebot
Copy link
Author

gissuebot commented Nov 1, 2014

Original comment posted by j...@nwsnet.de on 2012-07-31 at 10:35 AM


Yes, I'd like to see such an overload, too.

@gissuebot
Copy link
Author

Original comment posted by drothmaler on 2012-08-21 at 08:52 PM


Are there any news on this, kurt?

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2012-08-21 at 08:56 PM


FluentIterable.from(E...) or (E[])
Issues:
varargs suck, but there are lots of internal usages!
Why not just use asList(E...)? That’s been our go-to pushback argument for varargs and array-accepting overloads
Lots of users are using from(Enum.values())
gak to investigate all varargs methods.

Punting over to Greg...


Owner: gak@google.com

@gissuebot
Copy link
Author

Original comment posted by gak@google.com on 2012-08-21 at 09:06 PM


I'm ambivalent. I'll put it on the docket for our weekly discussion.

@gissuebot
Copy link
Author

Original comment posted by gak@google.com on 2012-08-21 at 09:17 PM


Oh, wait. No. I'm not. :-)

I occasionally get lured into thinking that varargs makes sense here, but it really doesn't. It's rare that you have a fixed set of elements that start a fluent chain of invocation since it's much more likely that the properties of those elements are known. Immutable*.of() are great for tests, but it'd be pretty strange to define test data as a FI.

As for the "working with legacy code" side of it, the clarity that comes from explicitly creating an immutable copy or a list view is quite valuable and encouraging callers to perform that operation as early as possible seems like a good thing too.

@gissuebot
Copy link
Author

gissuebot commented Nov 1, 2014

Original comment posted by j...@nwsnet.de on 2013-07-10 at 08:32 AM


Aside from Enum.values(), the reflection-related methods on class (getMethods(), getAnnotations() etc.) return arrays, and using FluentIterable to filter for specific elements helps here. Thus, FluentIterable.from(E...) would make things more concise.

@gissuebot
Copy link
Author

Original comment posted by gak@google.com on 2013-07-10 at 05:39 PM


I don't think that there's any disagreement that a method that accepts an array would be more concise for callers that are holding a reference to an array. Whether that is a sufficient benefit over forcing callers to explicitly copy or wrap that array is the question.

FluentIterable.from(Arrays.asList(clazz.getDeclaredMethods()))…
vs
FluentIterable.from(clazz.getDeclaredMethods())

or

FluentIterable.from(EnumSet.allOf(SomeEnum.class))…
vs
FluentIterable.from(SomeEnum.values())

The second versions are definitely more concise, but the cost is pretty minimal considering that the available adapters are pretty succinct.

@gissuebot
Copy link
Author

Original comment posted by lowasser@google.com on 2014-04-30 at 04:58 PM


Issue #1741 has been merged into this issue.

@gissuebot
Copy link
Author

gissuebot commented Nov 1, 2014

Original comment posted by lhun...@lyndir.com on 2014-04-30 at 05:14 PM


I understand there are alternative methods of achieving the same thing; though this change implies only a very minimal addition, is there a downside that is holding up this bug? Perhaps some form of discouraging array usage? (though it is not always a choice and varargs do have their benefits)

It seems only natural to me for this type of utility to accept both Iterables and Arrays as its purpose is mainly to iterate, and thus seems very in-line with the for construct. For arrays to not be supported almost feels like an inconsistency.

@gissuebot
Copy link
Author

Original comment posted by cgdecker@google.com on 2014-10-06 at 02:21 AM


FluentIterable.of(E[]) was added in 18.0.


Status: Fixed
Labels: Milestone-Release18

@scr
Copy link

scr commented Aug 21, 2015

Thanks for linking this to my PR, @cpovirk. I see some opinions of varargs, but not spelled out with reasoning.

Why was it preferred to add of(E[]) over of(E...) ? for methods under test that take an Iterable, why wouldn't you want to just call the method with FluentIterble.of(foo, bar, baz), which is much closer to the type that you want (Iterable)?

Yes, I get it there's always that method out there somewhere that will do the same thing - i.e. Arrays.asList(foo, bar, baz) also implements Iterable, but it's a mental hoop to remember it, or understand why it was used as a reviewer.

Certainly that mental-tax, however small, is worth not having to pay and choosing FluentIterble.of(foo, bar, baz) would be instantly intuitively obvious to everyone who sees it and stick in their minds more easily the next time they need to make an Iterable quickly for testing…

@scr
Copy link

scr commented Aug 21, 2015

I should point out that it was done this way for Iterators and I don't see why not also FluentIterable.

http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect/Iterators.html#forArray(T...)

@stephenh
Copy link

stephenh commented Sep 9, 2015

Somewhat frustrating this hasn't been added. @scr FWIW I've found https://github.com/jOOQ/jOOL to be pretty nice, and generally either a) have the methods/overloads I expect, or b) the maintainer is pretty open about adding methods/overloads as requested. FWIW/YMMV.

Unfortunately GWT doesn't have java.util.stream support yet, so I'm using old-school Guava transform/etc. with lambdas until I can just use jOOL for both client- and server-side.

@cpovirk cpovirk changed the title Factroy-Method to create a FluentIterable from an array Factory method to create a FluentIterable from varargs Sep 14, 2015
@cpovirk
Copy link
Member

cpovirk commented Sep 14, 2015

Reopening and retitling this to reflect where the discussion has gone.

@cpovirk cpovirk reopened this Sep 14, 2015
@cpovirk cpovirk removed this from the 18.0 milestone Sep 14, 2015
@cpovirk cpovirk changed the title Factory method to create a FluentIterable from varargs Factory method to create a FluentIterable from an array Sep 14, 2015
@cpovirk cpovirk added this to the 18.0 milestone Sep 14, 2015
@cpovirk
Copy link
Member

cpovirk commented Sep 14, 2015

No, wait. We already have an issue for varargs: #2136.

@cpovirk cpovirk closed this as completed Sep 14, 2015
@google google locked and limited conversation to collaborators Sep 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants