[12] RFR 8208483: Add 5 JNDI tests to com/sun/jndi/dns/FactoryTests/

[vyom tewari]

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