53 minutes ago, DividedByZero said:
I have made some progress (sort of).
I can input the name of the parent I want to list the hierarchy for and it does display the children through to the end, which is great.
But, if there is more than one child at a given point it will just list the first one and then grab its first child and so on.
void Graphics::SceneNodeTreeList(std::string name)
{
std::string szChild = "";
for (std::vector<SceneNode>::iterator it = sceneNodeVector.begin(); it != sceneNodeVector.end(); ++it)
{
if (it->name == name)
{
std::cout << it->name << "\r\n";
label:
for (std::vector<std::string>::iterator it2 = it->childVector.begin(); it2 != it->childVector.end(); ++it2)
{
// loop through immediate children
szChild = "";
for (unsigned n = 0; n < it2->size(); ++n)
szChild += it2->at(n);
std::cout << szChild << "\r\n";
// Does this have child and so on?
for (std::vector<SceneNode>::iterator it3 = sceneNodeVector.begin(); it3 != sceneNodeVector.end(); ++it3)
{
if (it3->name == szChild)
{
it = it3;
goto label; // Nasty, I know.
}
}
// Never makes it here due to goto statement
}
}
}
}
As I said, works fine if there is only ever once child at any given point, but skips past any subsequent child at that node.
Probably more of a logic issue I have here, but if anyone can give any suggestions that would be awesome.
Thanks again
There are some style and performance issues there, but I'll skip those as they'd be a bit off topic. I'll mention some things though that might make the code easier to read and understand:
- You might look into the 'auto' keyword. Among other things, it'd make your 'for' loops less verbose.
- Further, you might also look into range-based for loops, which can make these sorts of loops simpler still.
- Unless I'm misreading, this bit:
for (unsigned n = 0; n < it2->size(); ++n)
szChild += it2->at(n);
Appears to be adding one std::string instance to another. You should be able to do that more concisely and expressively using e.g. operator+=().
- I won't advise against using 'goto', as I don't want to create controversy. This sort of thing can certainly be implemented without it though, so I'd question whether it's really needed here.
- You may have a reason for storing all the nodes in a single vector and referencing children by name, but I'll just point out that you could instead reference children by pointer (in one form or another), which would eliminate the 'name lookup' step and might simplify the code. This would introduce some additional considerations, such as ownership, lifetime, pointer invalidation, and so on. But, it is an alternative to consider.
Beyond that, it might be worth starting with a recursive implementation, as Alberth was talking about. Once implemented, you could just stick with the recursive version if the trees aren't too deep, but regardless it might be a good starting point, as it can provide insight as to what other implementations (e.g. iterative) might look like.