Merge lp:~28msec/zorba/get-descendant-node-by-ordpath into lp:zorba

[Ghislain Fourny]

Introduced function OrdPathNode::getDescendantNodeByOrdPath (from Sausalito).

Introduced function OrdPathNode::getDescendantNodeByOrdPath (from Sausalito).

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/get-descendant-node-by-ordpath-2012-03-05T14-33-05.571Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job get-descendant-node-by-ordpath-2012-03-05T14-33-05.571Z is finished. The final status was:

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1. Got: 3 Pending.

[Matthias Brantner (matthias-brantner)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/get-descendant-node-by-ordpath-2012-03-05T20-26-03.63Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job get-descendant-node-by-ordpath-2012-03-05T20-26-03.63Z is finished. The final status was:

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1. Got: 1 Approve, 2 Pending.

[Markos Zaharioudakis (markos-za)]

Within the for loop:

for (i = 0; i < numAttrs; i++)

in the case of (pos == OrdPath::DESCENDANT) the function should return false, no?

Also, please use csize (instead of ulong) for storing the size of vectors and for iterating over vectors.

Also, in for loops, do ++i instead of i++. It's a very tiny optimization, but it doesn't cost anything so why let it pass....

[Ghislain Fourny (gislenius)]

Thanks Markos for these useful comments. The new revision addresses all three of them.

[Markos Zaharioudakis (markos-za)]

The numAttrs and numChildren local vars should also be csize instead of ulong, but I approve anyway.

review: Approve

[Ghislain Fourny (gislenius)]

Hi Markos, thanks, these two were updated as well before triggering the merge.

[Zorba Build Bot (zorba-buildbot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/get-descendant-node-by-ordpath-2012-03-13T11-52-05.311Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job get-descendant-node-by-ordpath-2012-03-13T11-52-05.311Z is finished. The final status was:

All tests succeeded!

=== modified file 'src/store/naive/node_items.cpp'
--- src/store/naive/node_items.cpp	2012-03-12 23:43:56 +0000
+++ src/store/naive/node_items.cpp	2012-03-13 10:31:28 +0000
@@ -1052,6 +1052,113 @@
   }
 }
 
+/*******************************************************************************
+
+********************************************************************************/
+bool
+OrdPathNode::getDescendantNodeByOrdPath(
+    const OrdPath& aOrdPath,
+    store::Item_t& aResult,
+    bool aAttribute) const
+{
+#ifdef TEXT_ORDPATH
+  if (getOrdPath() == aOrdPath)
+  {
+    aResult = this;
+    return true;
+  }
+#else
+  if (getNodeKind() == store::StoreConsts::textNode)
+  {
+    aResult = NULL;
+    return false;
+  }
+  else if (getOrdPath() == aOrdPath)
+  {
+    aResult = this;
+    return true;
+  }
+#endif
+
+  const XmlNode* parent = static_cast<const XmlNode*>(this);
+  csize i;
+
+  while (1)
+  {
+    if (parent->getNodeKind() != store::StoreConsts::documentNode &&
+        parent->getNodeKind() != store::StoreConsts::elementNode)
+    {
+      aResult = NULL;
+      return false;
+    }
+
+    if (aAttribute && parent->getNodeKind() == store::StoreConsts::elementNode)
+    {
+      const ElementNode* elemParent = reinterpret_cast<const ElementNode*>(parent);
+
+      csize numAttrs = elemParent->numAttrs();
+      for (i = 0; i < numAttrs; ++i)
+      {
+        AttributeNode* child = elemParent->getAttr(i);
+
+        OrdPath::RelativePosition pos =  child->getOrdPath().getRelativePosition(aOrdPath);
+
+        if (pos == OrdPath::SELF)
+        {
+          aResult = child;
+          return true;
+        }
+        else if (pos != OrdPath::FOLLOWING) // Includes DESCENDANT case
+        {
+          aResult = NULL;
+          return false;
+        }
+      }
+    }
+
+    const InternalNode* this2 = reinterpret_cast<const InternalNode*>(parent);
+
+    csize numChildren = this2->numChildren();
+    for (i = 0; i < numChildren; ++i)
+    {
+#ifdef TEXT_ORDPATH
+      OrdPathNode* child = static_cast<OrdPathNode*>(this2->getChild(i));
+#else
+      XmlNode* c = this2->getChild(i);
+
+      if (c->getNodeKind() == store::StoreConsts::textNode)
+        continue;
+
+      OrdPathNode* child = static_cast<OrdPathNode*>(c);
+#endif
+
+      OrdPath::RelativePosition pos =  child->getOrdPath().getRelativePosition(aOrdPath);
+
+      if (pos == OrdPath::SELF)
+      {
+        aResult = child;
+        return true;
+      }
+      else if (pos == OrdPath::DESCENDANT)
+      {
+        parent = child;
+        break;
+      }
+      else if (pos != OrdPath::FOLLOWING)
+      {
+        aResult = NULL;
+        return false;
+      }
+    }
+
+    if (i == numChildren)
+    {
+      aResult = NULL;
+      return false;
+    }
+  }
+}
+
 
 /*******************************************************************************
   Return true if "aOther" is an ancestore of "this".
 
=== modified file 'src/store/naive/node_items.h'
--- src/store/naive/node_items.h	2012-03-07 14:22:29 +0000
+++ src/store/naive/node_items.h	2012-03-13 10:31:28 +0000
@@ -664,6 +664,15 @@
         csize pos,
         store::StoreConsts::NodeKind nodeKind);
 
+  // Looks for a descendant node with the ordpath aOrdPath and puts it into the
+  // aResult variable. aAttribute specifies whether to look for an attribute or
+  // not.
+  bool
+  getDescendantNodeByOrdPath(
+      const OrdPath& aOrdPath,
+      store::Item_t& aResult,
+      bool aAttribute = false) const;
+
   virtual bool
   isAncestor(const store::Item_t&) const;