C++ iterating through a set [duplicate]

This question already has answers here: 开发者_如何学Python How to iterate std::set? (5 answers) Closed 6 years ago.

I recently changed some code to use a set instead of a vector:

std::set<b2Body *>toDestroy;
//std::vector<b2Body *>toDestroy;

But now I'm not sure how to iterate the set to find objects. This is what I had:

std::vector<b2Body *>::iterator pos2;
    for(pos2 = toDestroy.begin(); pos2 != toDestroy.end(); ++pos2) {
        b2Body *body = *pos2;     
        if (body->GetUserData() != NULL) {
            CCSprite *sprite = (CCSprite *) body->GetUserData();
            [self removeChild:sprite cleanup:YES];
        }
        _world->DestroyBody(body);
    }

What is the equivalent now that toDestroy is a set? Coming from Objective-C so I'm just learning best practices in C++.

EDIT: adding the error message I get:

error: no match for 'operator=' in 'pos2 = toDestroy. std::set<_Key, _Compare, _Alloc>::begin [with _Key = b2Body*, _Compare = std::less<b2Body*>, _Alloc = std::allocator<b2Body*>]()'

---

You need to declare your iterator to be a set iterator:

Change

std::vector<b2Body *>::iterator pos2;

to

std::set<b2Body *>::iterator pos2;

---

With C++11 you could simply write:

    for(auto pos2:toDestroy)

---

The iteration through the set will work as when the vector is there, so there is no need to change the code.

I would pay attention to what happen in DestroyBody (does the call remove the element from the vector or set, invalidating the iterator?)

Also the usage of vector, set or list depend on the usage:

• I would use a vector if the number of b2Body objects is known in advance so the capacity can be reserved in advance and new insertion deletion don't happen often, or if you need to access the vector's element randomly (without iterator)
• I would use the list if the number of elements is not known in advance and/or insertion and deletion happen often
• I would use the set if I need to iterate through an ordered list of element or for instance if I need the application to find if some data has already been processed and is in the set (the method find on the set is fast)

I think that the most appropriate container here is the list (and still the iteration code wouldn't need to be changed)

---

A lot of time it is worth type-defing templated container types (especially when used inside a class).

typedef std::set<b2Body *>   BodyCont;
//typedef std::vector<b2Body *>   BodyCont;
BodyCont                toDestroy;

Then your other code would have not needed modifying:

BodyCont::iterator pos2;

If you can make the typedef private then you know that implementation details are not escaping the class. If you need to make the typedef public then you know you are leaking implementation details and need to understand why and if you can tighten your design.

---

Since C++11, when iterating over a container, you should use "auto" to declare an iterator. In your case, the "for" line can be written like this: for(auto pos2 = toDestroy.begin(); pos2 != toDestroy.end(); ++pos2)