Code review request

Joe Bowbeer joe.bowbeer at gmail.com
Sat Feb 23 11:42:30 PST 2013


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/20130223/b2073b62/attachment.html 


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