[ 
http://nagoya.apache.org/jira/browse/XALANJ-667?page=comments#action_56811 ]
     
Bas de Bakker commented on XALANJ-667:
--------------------------------------

This is getting a bit off-topic, but I do not agree that equals() ought to 
compare the values of the objects.

For instance, I have frequently created a java.util.HashMap holding nodes where 
the desired equality was node identity.  But I have never needed a HashMap that 
compared node values.  So at least for me an equals() comparing node identity 
is much more useful.  (Yes, I realize this is not portable over DOM 
implementations.)

On a more theoretical line of reasoning, two nodes with the same value (i.e, 
isEqualNode() is true) are not necessarily "semantically equal".  They will 
usually have different parent nodes, sometimes different owner documents, will 
return different results from compareDocumentPosition(), etc.


> using object identity as a substitute for isSameNode is bad idea
> ----------------------------------------------------------------
>
>          Key: XALANJ-667
>          URL: http://nagoya.apache.org/jira/browse/XALANJ-667
>      Project: XalanJ2
>         Type: Improvement
>   Components: DTM
>     Versions: 2.0.0
>  Environment: Operating System: Other
> Platform: Other
>     Reporter: Andrei Tchijov
>     Assignee: Joe Kesselman

>
> In org.apache.xml.dtm.ref.dom2dtm.DOM2DTM.getHandleOfNode "==" used to
> test for two nodes been the same.  In the comment just before this method
> definition it is stated that this is "flaky" solution, and I can not agree 
> more.
>  The question is, what is wrong with using "equals" instead of "=="?  I do
> realize that "equals" is not part of the DOM, but same is true for "==".
> Andrei Tchijov
> PS These are snipets of code I am talking about
>   public int getHandleOfNode(Node node)
>   {
>     if (null != node)
>     {
>       // Is Node actually within the same document? If not, don't search!
>       // This would be easier if m_root was always the Document node, but
>       // we decided to allow wrapping a DTM around a subtree.
>         /*
>          * hyperNOC fix.
>          * used to be
>       if((m_root==node) ||
>          (m_root.getNodeType()==DOCUMENT_NODE &&
>           m_root==node.getOwnerDocument()) ||
>          (m_root.getNodeType()!=DOCUMENT_NODE &&
>           m_root.getOwnerDocument()==node.getOwnerDocument())
>          )
>          */
>       if((m_root==node) ||
>          (m_root.getNodeType()==DOCUMENT_NODE &&
>           m_root.equals( node.getOwnerDocument())) ||
>          (m_root.getNodeType()!=DOCUMENT_NODE &&
>           m_root.getOwnerDocument().equals( node.getOwnerDocument()))
>          )
>         {
>           // If node _is_ in m_root's tree, find its handle
>           //
>           // %OPT% This check may be improved significantly when DOM
>           // Level 3 nodeKey and relative-order tests become
>           // available!
>           for(Node cursor=node;
>               cursor!=null;
>               cursor=
>                 (cursor.getNodeType()!=ATTRIBUTE_NODE)
>                 ? cursor.getParentNode()
>                 : ((org.w3c.dom.Attr)cursor).getOwnerElement())
>             {
>                 /*
>                  * hyperNOC fix.
>                  * used to be
>               if(cursor==m_root)
>                  */
>               if(cursor.equals( m_root ))
>                 // We know this node; find its handle.
>                 return getHandleFromNode(node);
>             } // for ancestors of node
>         } // if node and m_root in same Document
>     } // if node!=null
>     return DTM.NULL;
>   }
> Same is true for getHandleFromNode
>   private int getHandleFromNode(Node node)
>   {
>     if (null != node)
>     {
>       int len = m_nodes.size();       
>       boolean isMore;
>       int i = 0;
>       do
>       {         
>         for (; i < len; i++)
>         {
>             /*
>              * hyperNOC fix
>              * used to be
>           if (m_nodes.elementAt(i) == node)
>              */
>           if (m_nodes.elementAt(i).equals( node ))
>             return i | m_dtmIdent;        
>         }
>         isMore = nextNode();
>  
>         len = m_nodes.size();
>            
>       }
>       while(isMore || i < len);
>     }
>    
>     return DTM.NULL;
>   }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to