Why is this loop only running once?

Question

Why is this loop only running once? noteDatabaseItem just takes a node and fills in the data. the xml has 3 notes in it.

XML:

<?xml version="1.0" encoding="utf-8"?>
<noteCollection>
  <note name="Test Note 1">This is test note 1 content!</note>
  <note name="Test Note 2">This is test note 2 content!</note>
  <note name="Test Note 3">This is test note 3 content!</note>
</noteCollection>

C++:

std::vector<notekeeper::noteDatabaseItem> noteList;
TiXmlElement* noteCollection = xmlDoc->FirstChildElement("noteCollection");
TiXmlElement* node = noteCollection->FirstChildElement("note");
int itemCount = 0;

while (node != NULL) {
    itemCount++;
    noteList.resize(itemCount);
    noteList.push_back(noteDatabaseItem(node));
    node = noteCollection->NextSiblingElement("note");
}

Why do you `resize` before you `push_back`? It resizes itself. Perhaps the last line should be `node = note->NextSiblingElement("note");`? — GManNickG, Oct 09 '10 at 22:09
The XML loaded is above and there is 3 notes so there isn't 1, and the resize is a mistake thanks. — Will03uk, Oct 09 '10 at 22:10
Hm, too bad I wasn't confident enough to make it an answer. :) — GManNickG, Oct 09 '10 at 22:14

score 9 · Accepted Answer · answered Oct 09 '10 at 22:13

9

Shouldn't it be node = node->NextSiblingElement("note")?

noteCollection has only children, not siblings, right?

answered Oct 09 '10 at 22:13

GôTô

7,974
3
32
43

score 3 · Answer 2 · answered Oct 09 '10 at 22:15

You're getting the wrong element in your loop. Try this:

while (node != NULL) {
    itemCount++;
    noteList.push_back(noteDatabaseItem(node));
    node = node->NextSiblingElement("note");
}

The next sibling of the current node is the one you want. You were trying to get the next sibling of the parent node.

score 1 · Answer 3 · answered Oct 09 '10 at 22:16

1

node = noteCollection->NextSiblingElement("note");

is meant to be

node = node->NextSiblingElement("note");

Stupid mistake. Sibling not Child.

answered Oct 09 '10 at 22:16

Will03uk

3,346
8
34
40

2

You should accept GoTo's answer, not re-post it. (i.e., delete this answer.) – GManNickG Oct 09 '10 at 22:17

Why is this loop only running once?

3 Answers3