RFR: 8013839: Enhance Logger API for handling of resource bundles

Mandy Chung mandy.chung at oracle.com
Thu Oct 3 19:29:07 UTC 2013


On 10/3/2013 8:56 AM, Daniel Fuchs wrote:
>
> Other methods (like getLogger) have @throws NPE in their javadoc.

True and I guess the NPE statement in the package summary was added 
afterwards.  I could see that it was meant to clean up the existing 
javadoc some days (something for the future if you wish).

>
>> 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.

Okay.

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

test/java/util/ResourceBundle/getBaseBundleName/TestGetBaseBundleName.java

  109                 return new Vector<String>(java.util.Arrays.asList(
  110                         new String[] {"dummy"})).elements();

Could you use Collections.enumeration?

test/java/util/logging/Logger/logrb/TestLogrbResourceBundle.java

  194             if (foobaz.getResourceBundleName() != null) {
  195                 throw new RuntimeException("Unexpected bundle: "
  196                         + foobar.getResourceBundle());
  197             }

I guess you meant to print foobar.getResourceBundleName() in line 196.

The test cases are good.  It'd be good to add some comments to
describe what each case verifies when appropriate.

You can make these changes before you push.  No need for a new webrev.

thanks
Mandy




More information about the core-libs-dev mailing list