Code review request (original) (raw)

Paul Sandoz paul.sandoz at oracle.com
Tue Feb 26 03:54:15 PST 2013

Hi Joe,

Thanks for the comments.

I have not resolved the more general comments about code style and tense.

On Feb 23, 2013, at 8:42 PM, 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

Fixed in the lambda repo.

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())); + }

Fixed.

On Feb 24, 2013, at 10:09 PM, Joe Bowbeer <joe.bowbeer at gmail.com> wrote:

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

Fixed.

3. StreamShape enum is missing its per-element javadoc

Fixed.

Paul.

More information about the lambda-libs-spec-observers mailing list