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