Code review request
Joe Bowbeer
joe.bowbeer at gmail.com
Sun Feb 24 13:09:45 PST 2013
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
More information about the lambda-libs-spec-experts
mailing list