Code review request
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-experts
mailing list