This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 241035 - Hints > General > Use Functional Operations should be "Warning on Current Line" by default
Summary: Hints > General > Use Functional Operations should be "Warning on Current Lin...
Status: NEW
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: 8.0
Hardware: PC Mac OS X
: P4 normal (vote)
Assignee: Svata Dedic
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-28 06:09 UTC by brettryan
Modified: 2016-09-16 10:46 UTC (History)
1 user (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description brettryan 2014-01-28 06:09:30 UTC
In many cases it's not ideal to convert for loops to functional stream operations as the result can be more verbose code than the original for loop.

Consider a situation which takes a collection and maps this to a string HashSet, the original for loop would look like the following:

    Set<String> ids = new HashSet<>();
    for (Widget w : getAllWidgets()) {
        ids.add(w.getId());
    }

After invoking the action the following is produced:

    Set<String> ids = new HashSet<>();
    getAllWidgets().stream().forEach((w) -> {
        ids.add(w.getId());
    });

Which if fully converted should actually look like the following:

    Set<String> ids = getAllWidgets().stream().forEach((w) -> {
        ids.add(w.getId());
    }).collect(Collectors.toCollection(HashSet::new));

In the end the code produced is larger and can lead newcomers to misinterpret the new result.
Comment 1 terje7601 2014-10-07 13:59:10 UTC
Actually, it should look like:

Set<String> ids = getAllWidgets().stream().map(Widget::getId).collect(toSet());

which is at least as small & clear as the original.
Comment 2 brettryan 2014-10-07 14:54:18 UTC
Very true, though as toSet() returns Set<T> it's not clear it's returning a HashSet (which it does), though its implementation could change. It could be said to be more explicit with:

Set<String> ids = getAllWidgets().stream().map(Widget::getId).collect(toCollection(HashSet::new));

The IDE could generate this form more generically as it could always identify the supplier collection.
Comment 3 terje7601 2014-10-07 16:04:51 UTC
Ah yes, I see your point now. If automatic conversion is provided (which in my opinion isn't the best solution, cf. issue 247772), it's definitely safer to do as you suggest, since toSet() & toList() provide no guarantees whatsoever (not even mutability, as I wrongly assumed).
Comment 4 gromit190 2016-09-16 09:18:02 UTC
Netbeans suggest that I edit the following:

    for (Double value : values) {
        sum += value;
    }


To:

    sum = values.stream().map((value) -> value).reduce(sum, (accumulator, _item) -> accumulator + _item);


Which is just plain obfuscating
Comment 5 brettryan 2016-09-16 10:46:38 UTC
(In reply to gromit190 from comment #4)
> Netbeans suggest that I edit the following:
> 
>     for (Double value : values) {
>         sum += value;
>     }
> 
> 
> To:
> 
>     sum = values.stream().map((value) -> value).reduce(sum, (accumulator,
> _item) -> accumulator + _item);
> 
> 
> Which is just plain obfuscating

I would say it'/ more of a lack of understanding the stream API, however, a simpler form would be

    double sum = values.stream().reduce(0.0, Double::sum);

You can use a mapToDouble but the result requires an additional step.

    double sum = values.mapToDouble(Double::doubleValue).sum();

Hints could then be enhanced to use one of the above for number types.

Note that Arrays.stream has overloads for primitive arrays which return a number based stream avoiding the need for the mapTo_