On 10/3/13 5:32 PM, Mandy Chung wrote:
Hi Daniel,

On 10/3/2013 7:47 AM, Daniel Fuchs wrote:

The new webrev is here:
http://cr.openjdk.java.net/~dfuchs/webrev_8013839/webrev.07/

Looks good.  Thanks for improving the javadoc.

line 1264  formatting nits - there are extra spaces that can be removed.

Right. Thanks for spotting that.

1892      * @throws NullPointerException if the given bundle is {@code
null}.

This is already captured in the package summary.  No need to specify this
@throws NPE.

Other methods (like getLogger) have @throws NPE in their javadoc.

setResourceBundle(bundle) forgets to check NPE (it should call
Objects.requireNonNull(bundle).

Line 1903 has this effect. I will add a comment to make it clear
that the NPE is intended.

I think it should move line 1913-1915
to the beginning after NPE check.   I think canReplaceResourceBundle
method body is simple that inlining in line 1908 would be cleaner and
still readable.

Right - done.

new webrev:
<http://cr.openjdk.java.net/~dfuchs/webrev_8013839/webrev.08/>

-- daniel

Mandy

Reply via email to