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