Add convenience collect methods to the Stream interface

[Rob Griffin (rgriffin)]

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 at quest.com 
Office +613-9811-8021  

-----Original Message-----
From: Remi Forax <forax at univ-mlv.fr> 
Sent: Tuesday, 11 December 2018 4:26 AM
To: Rob Griffin (rgriffin) <Rob.Griffin at quest.com>
Cc: core-libs-dev <core-libs-dev at openjdk.java.net>; Brian Goetz <brian.goetz at 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 at oracle.com>
> À: "Rob Griffin (rgriffin)" <Rob.Griffin at quest.com>, "core-libs-dev" 
> <core-libs-dev at 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 at quest.com