RFR (JAXP): 8054196: XPath: support any type

huizhe wang huizhe.wang at oracle.com
Thu Jan 22 00:45:37 UTC 2015


Thanks Lance for pointing me to Joe's -Xlint:all email thread. I 
re-compiled with -Xlint:all and noticed 7 warnings in this webrev. I've 
fixed them with a new webrev:
New webrev: http://cr.openjdk.java.net/~joehw/jdk9/8054196/webrev01/

Below are the details.
Refer to the previous webrev: 
http://cr.openjdk.java.net/~joehw/jdk9/8054196/webrev/

1. XPathExpressionImpl.java:143 and XPathImpl.java:213

    These warnings are fixed by having the getXPathResult method in 
XPathImpUtil returning the type instead

2. XPathExpressionImpl.java:165: warning: [rawtypes] found raw type: 
XPathEvaluationResult
     similarly, XPathImpl.java:222 and XPathImpl.java:235

     Changed to return XPathEvaluationResult<?>

3. XPath.java:359, XPath.java:454, XPathExpression.java:252 and 
XPathExpression.java:344
    These are warnings from the default methods, casting the result of 
existing (old) methods. Use type.cast instead.

4. XPathNodesImpl.java:97: warning: [cast] redundant cast to Node
     Removed the cast

5. XPathResultImpl.java:160: warning: [fallthrough] possible 
fall-through into case
     Added break;

Thanks,
Joe

On 1/20/2015 10:51 AM, huizhe wang wrote:
>
> On 1/20/2015 7:02 AM, Lance Andersen wrote:
>> Hi Joe,
>>
>> I think the changes look fine.
>
> Thanks.
>>
>> I am wondering if we have any suggested standard for the use of 
>> @Override  as I see it is inconsistent in its usage with methods 
>> implementing an interface.  Is this something we should add to 
>> existing code?  I don't see Netbeans asking for it to be done except 
>> in the case of an overridden method?
>
> NetBeans does make a suggestion such as: "Add @Override Annotation".  
> I think you're right about avoiding comment duplication. In case of 
> existing "evaluate" methods, I previously updated* the javadocs for 
> both the interface and impl. It makes sense to take advantage of the 
> "automatically inherit" feature 
> <http://docs.oracle.com/javase/7/docs/technotes/tools/solaris/javadoc.html#inheritingcomments> 
> of the javadoc tool to avoid the duplication.
>
> *Note that the update was only on format and styles, and some 
> re-wording, no changes on definition or semantics.
>
>>
>> Thank you for the extra clean up Joe
>
> Thank you, now it looks better and cleaner.
>
> Best,
> Joe
>
>>
>> Best
>> Lance
>> On Jan 16, 2015, at 9:33 PM, huizhe wang <huizhe.wang at oracle.com 
>> <mailto:huizhe.wang at oracle.com>> wrote:
>>
>>>
>>> On 1/16/2015 1:29 PM, Lance Andersen wrote:
>>>> Hi Joe,
>>>>
>>>>
>>>>
>>>> Overall it is OK, a few minor comments
>>>>
>>>> -  Is there a reason that XPathExpressionImpl  is no longer public 
>>>> and XPathImpl is public?
>>>
>>> Ok, I'll keep both public, may be useful in the future.
>>>
>>>> - I think you can leverage {@inheritdoc} in your impl classes to 
>>>> avoid comment duplication possibly?
>>>
>>> Looks like javadoc "Automatically inherit comment ". So @Override is 
>>> good enough. I've removed the javadocs for methods that override, 
>>> including the existing methods. It's good to avoid the duplication, 
>>> and potential copy n paste errors.
>>>
>>>> - please remember to make the copyright year 2015
>>>
>>> Done.
>>>
>>>> - XPathTestBase, can the static block be moved to a @BeforeClass
>>>
>>> Moved to within the Dataprovider.
>>>
>>>> - XPathNodes, should that have an @since 1.9?
>>>
>>> Fixed.
>>>
>>>> - Given you are not including @param, etc for your test comments, 
>>>> you might want to consider /* */ vs /** */ style comments.
>>>
>>> Looks like s/\/**/\/* worked the trick.
>>>
>>>>  That is consider if you really want doc comments (really a style 
>>>> choice but some IDEs will issue a warning for missing tags
>>>
>>> Yeh, it's good to turn off the warning "light" :-)
>>>
>>> http://cr.openjdk.java.net/~joehw/jdk9/8054196/webrev/
>>>
>>> Thanks,
>>> Joe
>>>
>>>>
>>>>
>>>> HTH
>>>>
>>>> Best,
>>>> Lance
>>>> On Jan 16, 2015, at 2:51 PM, huizhe wang <huizhe.wang at oracle.com 
>>>> <mailto:huizhe.wang at oracle.com>> wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> Could you review the change?
>>>>>
>>>>> Thanks,
>>>>> Joe
>>>>>
>>>>> On 12/18/2014 1:24 PM, huizhe wang wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This is to add support for any type and improvement with new 
>>>>>> features reflected in the new evaluateExpression methods, 
>>>>>> XPathEvaluationResult and XPathNodes.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8054196
>>>>>> http://cr.openjdk.java.net/~joehw/jdk9/8054196/webrev/
>>>>>>
>>>>>> Thanks,
>>>>>> Joe
>>>>>
>>>>
>>>> <Mail Attachment.gif> 
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> 
>>>>
>>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
>>>> Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>> Oracle Java Engineering
>>>> 1 Network Drive
>>>> Burlington, MA 01803
>>>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>>>>
>>>>
>>>>
>>>
>>
>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> 
>>
>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
>> Andersen| Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>>
>>
>>
>




More information about the core-libs-dev mailing list