(original) (raw)

vyom tewari vyom.tewari at oracle.com
Mon Aug 6 08:12:29 UTC 2018

Previous message: [httpclient] HTTP2: Memory Leak with Proxy
Next message: [12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Chris,

1-> DirAFactory.java, "getIbjectInstance" returns "null" if it fails to construct object. I will suggest you to throw some "RuntimeException" instead returning null. If you return null then caller of "getObjectInstance" had to check for null and it will end in lots of boilerplate code.

2-> DirTxtFactory.java same as "DirFactory.java".

3-> In LookupWithAnyAttrProp/LookupWithAttrProp java create a constant for "1.2.4.1" and put one liner comment if possible. This is will help future maintainer to understand why we are comparing with this value.

One generic comment, in most of the places i can see, you declared functions to throw generic exception "Exception", please change it to specific exception or list of specific exceptions if possible.

Thanks,

Vyom

On Monday 30 July 2018 02:08 PM, Chris Yin wrote:

Please review the changes to add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/ in OpenJDK, thanks

bug: https://bugs.openjdk.java.net/browse/JDK-8208483 <https://bugs.openjdk.java.net/browse/JDK-8208483> webrev: http://cr.openjdk.java.net/~xyin/8208483/webrev.00/ <http://cr.openjdk.java.net/~xyin/8208483/webrev.00/> Regards, Chris

Previous message: [httpclient] HTTP2: Memory Leak with Proxy
Next message: [12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the core-libs-dev mailing list