Code review request

[Joe Bowbeer]

A few more comments.

1. General:

The method descriptions should be written 3rd person declarative, according
to Oracle's style guide

http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide

This is not followed in many places.  For example:

Get the {@code StreamShape} describing the input shape of the pipeline

=>
Gets the {@code StreamShape} describing the input shape of the pipeline.


2. Typo (missing space) in PipelineHelper javadoc:

40  * the last intermediate operation described by this {@code
PipelineHelper}.The


3. StreamShape enum is missing its per-element javadoc

--Joe


On Sat, Feb 23, 2013 at 11:42 AM, Joe Bowbeer <joe.bowbeer at gmail.com> wrote:

> We should send these comments in emails?  I don't see a way to comment at
> the link provided.
>
> I repeat some of Remi's comments regarding formatting below.
>
> File:
>
>
> http://cr.openjdk.java.net/~briangoetz/jdk-8008670/webrev/src/share/classes/java/util/Map.java.patch
>
> 1. Please run this through a code formatter to conform with Oracle's
> standard.  Things to fix:
>
> parameter wrapping should indent only 8 spaces:
>
> + default V merge(K key, V value,
> +                 BiFunction<? super V, ? super V, ? extends V>
> remappingFunction) {
>
> if-else brace should be on same line:
>
> + }
> + else if ((newValue = remappingFunction.apply(oldValue, value)) != null) {
>
> multi-line 'if' always needs braces?
>
> + if (replace(key, oldValue, newValue))
> +     return newValue;
>
>
> 2. replaceAll javadoc: Function#map => Function#apply
>
> calling the function's {@code Function#map} method
>
> =>
> calling the function's {@code Function#apply} method
>
>
> 3. replaceAll question
>
> What's with all the finals?
>
> +        final Iterator<Map.Entry<K, V>> entries = entrySet().iterator();
> +        while (entries.hasNext()) {
> +            final Map.Entry<K, V> entry = entries.next();
> +            entry.setValue(function.apply(entry.getKey(),
> entry.getValue()));
> +        }
>
> Why not code this as follows, just like forEach?
>
> +        for (Map.Entry<K, V> entry : entrySet()) {
> +            entry.setValue(function.apply(entry.getKey(),
> entry.getValue()));
> +        }
>
> --Joe
>
>
> On Thu, Feb 21, 2013 at 11:17 AM, Brian Goetz <brian.goetz at oracle.com>wrote:
>
>> At
>>   http://cr.openjdk.java.net/~**briangoetz/jdk-8008670/webrev/<http://cr.openjdk.java.net/~briangoetz/jdk-8008670/webrev/>
>>
>> I've posted a webrev for about half the classes in java.util.stream. None
>> of these are public classes, so there are no public API issues here, but
>> plenty of internal API issues, naming issues (ooh, a bikeshed), and code
>> quality issues.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/attachments/20130224/38adfc5b/attachment.html