DO NOT REPLY [Bug 4808] New: - using object identity as a substitute for isSameNode is bad idea

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=4808>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=4808

using object identity as a substitute for isSameNode is bad idea

           Summary: using object identity as a substitute for isSameNode is
                    bad idea
           Product: XalanJ2
           Version: 2.0.0
          Platform: Other
        OS/Version: Other
            Status: NEW
          Severity: Major
          Priority: Other
         Component: org.apache.xml.dtm
        AssignedTo: xalan-dev@xml.apache.org
        ReportedBy: andrei.tchijov@hyperNOC.com
                CC: andrei.tchijov@hyperNOC.com


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;
  }