RE: Add convenience collect methods to the Stream interface

10 Dec 2018

      Hi Remi,

There are certainly places where we could do this when we are simply iterating over the results but that is not always the case. However I was disappointed to find that the enhanced for loop can't iterate over a stream so if callers of your example methods where doing something like this

for (Employee emp : getAllEmployee()) {
  ...
}

then it would have to change to a forEach call if getAllEmployee returned a Stream. 

Regards,

Rob Griffin
Software Analyst, Spotlight on SQL Server
Quest | R&D
rob.griffin@quest.com 
Office +613-9811-8021  

-----Original Message-----
From: Remi Forax <forax@univ-mlv.fr> 
Sent: Tuesday, 11 December 2018 4:26 AM
To: Rob Griffin (rgriffin) <Rob.Griffin@quest.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Brian Goetz <brian.goetz@oracle.com>
Subject: Re: Add convenience collect methods to the Stream interface

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Hi Rob,
i will add to the answer of Brian that if you have too many .collect(toList()), it's perhaps your application perhaps suffers of the equivalent of the n + 1 select query you have with SQL but with the Stream API.

You should try to see if returning a Stream instead of a List for some of methods is not a better option:
  public List<Employee> getAllEmployee() {
    return employees.stream().filter(Employee::isActive).collect(toList());
  }
  public List<Employee> getManager(List<Employee> list) {
    return list.stream().filter(Employee::isManager).collect(toList());
  }
  ...
  getManager(getAllEmployee());

should be:
  public Stream<Employee> getAllEmployee() {
    return employees.stream().filter(Employee::isActive);
  }
  public Stream<Employee> getManager(Stream<Employee> stream) {
    return stream.filter(Employee::isManager);
  }
  ...
  getManager(getAllEmployee()).collect(toList());

regards,
Rémi

----- Mail original -----
...
De: "Brian Goetz" <brian.goetz@oracle.com>
À: "Rob Griffin (rgriffin)" <Rob.Griffin@quest.com>, "core-libs-dev" 
<core-libs-dev@openjdk.java.net>
Envoyé: Lundi 10 Décembre 2018 17:14:41
Objet: Re: Add convenience collect methods to the Stream interface
...
As will surprise no one, this was extensively discussed during the 
development of the Streams API.  (Our default position on "convenience 
methods" is hostile.  While everyone sees the benefit of convenience 
methods (it's convenient!), most people don't see the cost, which 
includes the complexity for users to understand the model by looking 
at the API; having lots of ad-hoc convenience method obscures the 
underlying model, making it harder for everyone to learn or reason 
about.  That default position seems to stand up pretty well here, as 
the stream API is pretty well factored.)
Let's be honest: the "convenience" or concision of being able to say
.toList() instead of .collect(toList()) is really small.  I don't 
think you'll be able to justify it by saying "but we do it a lot."
(Digression: to whoever is about to say "then why `toArray()`?  Arrays 
are different; for better or worse, they're part of the language, and 
they lend themselves particularly poorly to the Collector API, and 
there are particular parallelization optimizations that are possible 
for arrays that can't be accessed through Collector.  End digression.)
It is possible, however, that one could justify `toList()` on the 
basis of _discoverability_.  (Though I'm having a hard time seeing any 
world where `toSet()` makes the cut.)  New users who approach streams 
will not easily be able to figure out how to materialize a list from a 
stream, even though this is an entirely reasonable and quite common 
thing to want to do.  Having to learn about `collect()` first is 
asking a lot of users who are still wrapping their heads around 
streams.  Not only would `toList()` be more discoverable, it would 
provide a path to discovery of the rest of the `collect()` API.  This is a point in its favor.
A significant downside of adding `toList()` is that by diluting the 
orthogonality of the existing API, it provides both incentive and 
justification for further dilution, leading to someplace we don't want 
to be.  (And, the cost of that falls heavily on the stewards, which in 
turn takes time away from far more valuable activities.)
Put it this way: imagine we have a budget of one convenience method in 
Stream for every five years.  Is this the one we want to spend the 
next five year's budget on?  (And, who of the proponents will 
volunteer to answer the next 200 "I have an idea for a stream method" 
mails, explaining that the budget is spent?)
So, summary:
- I won't outright veto `toList`, as I would for almost all other 
"convenience" streams additions, because this one actually has a valid 
non-convenience argument;
 - But, it's still not a slam dunk.
On 12/9/2018 5:44 PM, Rob Griffin (rgriffin) wrote:
...
Hi,
I have raised an enhancement request (Incident Report 913453) about 
adding some convenience methods to the Stream interface that collect 
the stream and Pallavi Sonal asked me to start a thread here about that.
More than 50% of our Stream collect calls use Collectors.toList() or
Collectors.toSet() as arguments so I think it would be very handy if 
the Stream interface had default collectToList and collectToList and collectToMap methods.
The advantages are:
     it would be easier to use code completion in IDEs. There are lot of classes
     starting with Collect so finding the Collectors class is a bit of a pain.
     one less method call in what is usually a long chain of calls.
Regards,
Rob Griffin
Software Analyst, Spotlight on SQL Server Quest | R&D 
rob.griffin@quest.com