CODEWITHSUNDEEP
c++performance
8-bitButterfly
8-bitButterfly

Reputation: 203

Improving simplicity with std::list

What are some better (cleaner, more readable and/or efficient) ways of doing this:

std::list<Fruit*>   Apples;
std::list<Fruit>    Basket;

for (std::list<Fruit*>::iterator niApple(Apples.begin());
     niApple != Apples.end(); niApple++) {

    for (std::list<Fruit>::iterator niBasket(Basket.begin());
         niBasket != Basket.end(); niBasket++) {

        if (&(*niBasket) == *niApple) {
            Basket.erase(niBasket);
            break;
        }

    } // loop

} // loop

What would you recommend? I primarly need handles to the Apples which i'll be placing inside the Basket, so to remove the Apples form the Basket without having to search (e.g.. by index inside fixed array). However, the Basket needs to be allocating and deallocating the memory in the process.

Upvotes: 4

Views: 156

Answers (2)

Guilherme Bernal
Guilherme Bernal

Reputation: 8293

Another C++11 way:

list<Fruit*> Apples;
list<Fruit>  Basket;

Basket.remove_if([](const Fruit& fruit) {
    return any_of(Apples.begin(), Apples.end(), [](Fruit* apple) {
        return &fruit == apple;
    });
});

Now changing the first container to hold iterators to the second one:

list<list<Fruit>::iterator> Apples;
list<Fruit>  Basket;

for (auto apple : Apples)
    Basket.erase(apple);

This way you get better performance and little or no change to your interface, as iterators behave like pointers in most cases.

Also take a look at this: Should std::list be deprecated?

Note that for both solutions to work, the Basket container must be std::list.

Upvotes: 7

Mooing Duck
Mooing Duck

Reputation: 66922

std::list<Fruit*>   Apples;
std::list<Fruit>    Basket;

auto is_apple = [](const Fruit& frt) { 
        for(auto apl: Apples) { //for each Apple pointer
            if (apl== &frt) //if it points at the current fruit
                return true; //then it's one of the apples
        }
        return false; //otherwise it's not one of the apples
    };
Basket.remove_if(is_apple);

That seems simpler to me. (Hooray C++11!)

Upvotes: 4

Related Questions