JDK 8 Code Review Request for 7195301: XML Signature DOM implementation should not use instanceof to determine type of Node (original) (raw)
Xuelei Fan xuelei.fan at oracle.com
Sat Sep 8 03:45:10 UTC 2012
- Previous message (by thread): JDK 8 Code Review Request for 7195301: XML Signature DOM implementation should not use instanceof to determine type of Node
- Next message (by thread): JDK 8 Code Review Request for 7195301: XML Signature DOM implementation should not use instanceof to determine type of Node
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Looks fine to me.
It takes me to think whether it is safe to cast an object of ELEMENT_NODE type to org.w3c.dom.Element.
For example, in RetrievalMethodResolver.java
281 static Element getDocumentElement(Set set) { 282 Iterator it=set.iterator(); 283 Element e=null; 284 while (it.hasNext()) { 285 Node currentNode=it.next(); -286 if (currentNode instanceof Element) { +286 if (currentNode != null && currentNode.getNodeType() == Node.ELEMENT_NODE) { 287 e=(Element)currentNode; 288 break; 289 } 290 291 }
It is safe because the element is come from the org.w3c.dom.Node. Using "instanceof" looks more straightforward. Maybe you want to use the same style to check the node type.
The same for CanonicalizerBase.java,
Anyway, it is just a very very minor concern. The fix is fine to me.
Xuelei
On 9/7/2012 10:39 PM, Sean Mullan wrote:
Hi Xuelei,
Would you be able to review my fix for 7195301? webrev: http://cr.openjdk.java.net/~mullan/webrevs/7195301/webrev.00/ No regression test (noreg-hard), requires complex setup, but I have attached a test to the bug report that can be run manually. Thanks, Sean
- Previous message (by thread): JDK 8 Code Review Request for 7195301: XML Signature DOM implementation should not use instanceof to determine type of Node
- Next message (by thread): JDK 8 Code Review Request for 7195301: XML Signature DOM implementation should not use instanceof to determine type of Node
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]