Brent Sqar
Brent Sqar

Reputation: 5

STL <list> generate and delete items properly

  1. I wonder, if the objects generated with "new" are deleted correctly with this concept.
  2. I fail to use "bind" instead of "bind2nd". How do I do this?
  3. Last question: how do I do the same with a lambda term instead of a named function?

Why do I use list? I decided to take a list instead of a vector because my objects are not sequenced. At the very end I want to bring some "waterfall like" code to object oriented, basically because of performance issues (you will see the problems here), so I need a concept, where I have fast access to objects in hierarchies of containers and let them communicate with each other, and that is fast too. Maybe think of a modular synthesizer.

class Layer {
    private:
    string name;
    bool active;
    public:
    Layer();
    Layer(string m_name, bool m_active);
    bool isName(string m_name);
};

// ... constructors

bool Layer::isName(string m_name) {
return name == m_name;
}

class Stack {
    public:
    list<Layer*> layer;
    list<Layer*>::iterator iter;
};

int main() {
Stack stack;
stack.layer.push_back(new Layer);
stack.layer.push_back(new Layer("snail", true));
stack.layer.push_back(new Layer("squirrel", false));

string search = "snail";
stack.layer.remove_if(bind2nd(mem_fun(&Layer::isName), search));

return 0;
}

Upvotes: 0

Views: 208

Answers (1)

Jerry Coffin
Jerry Coffin

Reputation: 490278

No, you're not managing the memory correctly. Unless you need to do otherwise, I'd expunge most (if not all) the pointers from the code:

class Stack {
    public:
    list<Layer> layer;
    list<Layer>::iterator iter;
};

Then the memory manage becomes pretty much automatic:

Stack stack;
stack.layer.push_back(Layer("snail", true));
stack.layer.push_back(Layer("squirrel", false));

If you have a new enough compiler, you probably want to replace push_back with emplace_back though.

For something like this, a lambda is almost certainly better than std::bind:

stack.layer.remove_if([](Layer const &s) { return s.isName(search); }

As an aside, you might also want to change isName to take a reference to a const string instead of a string (saves copying the string every call).

From what you're showing, I'd also consider using std::map instead of std:list. It'll support lookups (for one thing) more directly.

Edit: one other point I should mention is that in a case like this where there's one field that you'll almost certainly use as the "key" essentially all the time (the name, in this case) you may want to overload operator<, operator==, etc., to work on that key field:

class Layer { 
    // ...
    bool operator<(Layer const &other) { return name < other.name; }
    bool operator==(Layer const &other) { return name == other.name; }
};

With this, operations like sorting, removal, etc., become even simpler. The one above becomes just:

 stack.layer.remove(search);

Upvotes: 7

Related Questions