C++ Iterating over a list of a certain subclass

Question

I have two classes. The superclass is a "Component" class, and the subclass is a "Transform" class.

The framework I'm using has a function that returns a list of components of a certain type. However, the list will return them as Component, since the type isn't restricted to a specific subclass (however it's the way I'm using it).

So, in the following scenario, I know that all the returned components will be of the Transform subclass. What I'm doing is I'm iterating over the list and then casting each component to Transform. Here is my code:

std::list<Cistron::Component*,std::allocator<Cistron::Component*>> playerTransforms = objectManager->getComponents(player,"Transform");
std::list<Cistron::Component*>::iterator playerComponentIterator = playerTransforms.begin();
for (playerComponentIterator; playerComponentIterator != playerTransforms.end(); playerComponentIterator++)
{
    Transform *tmpTransform = static_cast<Transform*> (*playerComponentIterator);
    std::cout << tmpTransform->x ;
    std::cout << tmpTransform->y ;
}

How efficient is this? I'm quite new to C++, so I have no idea if there's a better way of doing this.

Evgeny Lazin · Accepted Answer · 2013-11-03T13:46:32.657

2

This isn't a good design, your compiler should generate a warning in this case. Normally, you should upcast your pointer using dynamic_cast. This cast has some runtime cost - aproximately the same as virtual method call but it will generate exception if you try to cast incompatible pointers. Try to redesign your app to eliminate this code. You should only call virtual methods of the Component class, you shouldn't cast pointer to Component to pointer to Transform. This thing indicate bad design.

One possible desigion is to make getComponents a template method to eliminate cast:

template<class T>
list<T*> getComponents(Player* player, std::string name) {
    ...
}

or maybe just this:

list<Transform*> getTransfromComponents(Player* player) {...}

In a case when you can't refactor this code, you can always transform your list:

    list<Component*> rlist = ...
    list<Transform*> llist;
    // Upcast all
    transform(rlist.begin(), 
              rlist.end(), 
              back_inserter(llist), 
              [](Component* r) {
                  return dynamic_cast<Transform*>(r);
    });
    // Remove all null values
    llist.remove(nullptr);

edited Nov 03 '13 at 13:46

answered Nov 03 '13 at 12:54

Evgeny Lazin

9,193
6
47
83

1

Trying to cast an incompatible pointer will return a null pointer. Trying to cast an incompatible reference will throw an exception. – Oswald Nov 03 '13 at 12:55
Good point! But I can't remember when I need to use dynamic_cast in real code last time, anyway :) – Evgeny Lazin Nov 03 '13 at 12:57
Let's say I have around 20 subclasses of Component each with their own functions and variables. Wouldn't this lead to a bloated base class full of unneeded things? – Pat Nov 03 '13 at 12:57
This will lead to code bloat, my point is that you need to redisign your app to elimitate the need of cast. Your query transfrom objects but your method returns pointer to some base class. Why not *void? :) – Evgeny Lazin Nov 03 '13 at 13:06
I didn't make the method. It's in a library I'm using. (Cistron) – Pat Nov 03 '13 at 13:12
Thank you for answering again! That last part seems a bit.. performance heavy? (I wouldn't know as I come from a Java background.) The library I'm using is open source. I'll see if I can do a template method, and maybe even bypass the performance hit that LihO mentioned. I don't know if I'm experienced enough in C++ to do it.. – Pat Nov 03 '13 at 15:04
This code introduces a space leak, because we need full size temporary to store `Transform*` values. Another possibility is to cast values as they needed. This can be done the way you do this or you can create wrapper around list type that will cast every value on demand (you need to implement custom iterators for this, this is not trivial). Are you shure that this code realy needs optimization? – Evgeny Lazin Nov 03 '13 at 15:26
This was my question! Heh. Though I'm currently looking more into the framework, and I see they have an event messaging system, which I think will be a lot better to use than iterating over the list, since LihO also mentioned it will be slow to do so.. – Pat Nov 04 '13 at 07:20

score 1 · Answer 2 · edited May 23 '17 at 10:32

1

The std::list is usually implemented as double-linked list, which means that elements are scattered through the memory, which means that iterating through it is slow. Check: Why is it so slow iterating over a big std::list?

But what I would worry more about is the use of reflection:

objectManager->getComponents(player,"Transform");

that might actually be the real bottleneck of this piece of code.

edited May 23 '17 at 10:32

Community

1
1

answered Nov 03 '13 at 12:59

LihO

41,190
11
99
167

You're right it's a bottleneck. There's a similar function in Unity3D and they warn there about its performance cost. However the framework only provides this method to to retrieve the components in this case. – Pat Nov 03 '13 at 13:10

C++ Iterating over a list of a certain subclass

2 Answers2