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.w...@oracle.com
<mailto:huizhe.w...@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.w...@oracle.com
<mailto:huizhe.w...@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.ander...@oracle.com <mailto:lance.ander...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
