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

Proposed by Ghislain Fourny on 2012-03-05
Status: Merged
Approved by: Markos Zaharioudakis on 2012-03-13
Approved revision: 10698
Merged at revision: 10719
Proposed branch: lp:~28msec/zorba/get-descendant-node-by-ordpath
Merge into: lp:zorba
Diff against target: 136 lines (+116/-0)
2 files modified
src/store/naive/node_items.cpp (+107/-0)
src/store/naive/node_items.h (+9/-0)
To merge this branch: bzr merge lp:~28msec/zorba/get-descendant-node-by-ordpath
Reviewer Review Type Date Requested Status
Markos Zaharioudakis 2012-03-05 Approve on 2012-03-12
Matthias Brantner 2012-03-05 Approve on 2012-03-05
Review via email: mp+95902@code.launchpad.net

Commit message

Introduced function OrdPathNode::getDescendantNodeByOrdPath (from Sausalito).

Description of the change

Introduced function OrdPathNode::getDescendantNodeByOrdPath (from Sausalito).

To post a comment you must log in.
Zorba Build Bot (zorba-buildbot) wrote :

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) wrote :

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

review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

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) wrote :

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

Markos Zaharioudakis (markos-za) wrote :

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) wrote :

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

Markos Zaharioudakis (markos-za) wrote :

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

review: Approve
Ghislain Fourny (gislenius) wrote :

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

Zorba Build Bot (zorba-buildbot) wrote :

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) wrote :

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

All tests succeeded!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/store/naive/node_items.cpp'
2--- src/store/naive/node_items.cpp 2012-03-12 23:43:56 +0000
3+++ src/store/naive/node_items.cpp 2012-03-13 10:31:28 +0000
4@@ -1052,6 +1052,113 @@
5 }
6 }
7
8+/*******************************************************************************
9+
10+********************************************************************************/
11+bool
12+OrdPathNode::getDescendantNodeByOrdPath(
13+ const OrdPath& aOrdPath,
14+ store::Item_t& aResult,
15+ bool aAttribute) const
16+{
17+#ifdef TEXT_ORDPATH
18+ if (getOrdPath() == aOrdPath)
19+ {
20+ aResult = this;
21+ return true;
22+ }
23+#else
24+ if (getNodeKind() == store::StoreConsts::textNode)
25+ {
26+ aResult = NULL;
27+ return false;
28+ }
29+ else if (getOrdPath() == aOrdPath)
30+ {
31+ aResult = this;
32+ return true;
33+ }
34+#endif
35+
36+ const XmlNode* parent = static_cast<const XmlNode*>(this);
37+ csize i;
38+
39+ while (1)
40+ {
41+ if (parent->getNodeKind() != store::StoreConsts::documentNode &&
42+ parent->getNodeKind() != store::StoreConsts::elementNode)
43+ {
44+ aResult = NULL;
45+ return false;
46+ }
47+
48+ if (aAttribute && parent->getNodeKind() == store::StoreConsts::elementNode)
49+ {
50+ const ElementNode* elemParent = reinterpret_cast<const ElementNode*>(parent);
51+
52+ csize numAttrs = elemParent->numAttrs();
53+ for (i = 0; i < numAttrs; ++i)
54+ {
55+ AttributeNode* child = elemParent->getAttr(i);
56+
57+ OrdPath::RelativePosition pos = child->getOrdPath().getRelativePosition(aOrdPath);
58+
59+ if (pos == OrdPath::SELF)
60+ {
61+ aResult = child;
62+ return true;
63+ }
64+ else if (pos != OrdPath::FOLLOWING) // Includes DESCENDANT case
65+ {
66+ aResult = NULL;
67+ return false;
68+ }
69+ }
70+ }
71+
72+ const InternalNode* this2 = reinterpret_cast<const InternalNode*>(parent);
73+
74+ csize numChildren = this2->numChildren();
75+ for (i = 0; i < numChildren; ++i)
76+ {
77+#ifdef TEXT_ORDPATH
78+ OrdPathNode* child = static_cast<OrdPathNode*>(this2->getChild(i));
79+#else
80+ XmlNode* c = this2->getChild(i);
81+
82+ if (c->getNodeKind() == store::StoreConsts::textNode)
83+ continue;
84+
85+ OrdPathNode* child = static_cast<OrdPathNode*>(c);
86+#endif
87+
88+ OrdPath::RelativePosition pos = child->getOrdPath().getRelativePosition(aOrdPath);
89+
90+ if (pos == OrdPath::SELF)
91+ {
92+ aResult = child;
93+ return true;
94+ }
95+ else if (pos == OrdPath::DESCENDANT)
96+ {
97+ parent = child;
98+ break;
99+ }
100+ else if (pos != OrdPath::FOLLOWING)
101+ {
102+ aResult = NULL;
103+ return false;
104+ }
105+ }
106+
107+ if (i == numChildren)
108+ {
109+ aResult = NULL;
110+ return false;
111+ }
112+ }
113+}
114+
115
116 /*******************************************************************************
117 Return true if "aOther" is an ancestore of "this".
118
119=== modified file 'src/store/naive/node_items.h'
120--- src/store/naive/node_items.h 2012-03-07 14:22:29 +0000
121+++ src/store/naive/node_items.h 2012-03-13 10:31:28 +0000
122@@ -664,6 +664,15 @@
123 csize pos,
124 store::StoreConsts::NodeKind nodeKind);
125
126+ // Looks for a descendant node with the ordpath aOrdPath and puts it into the
127+ // aResult variable. aAttribute specifies whether to look for an attribute or
128+ // not.
129+ bool
130+ getDescendantNodeByOrdPath(
131+ const OrdPath& aOrdPath,
132+ store::Item_t& aResult,
133+ bool aAttribute = false) const;
134+
135 virtual bool
136 isAncestor(const store::Item_t&) const;
137

Subscribers

People subscribed via source and target branches