Is it possible to avoid managing memory manually in this situation in c++?

  • A+
Category:Languages

I have a Storage class that keeps a list of Things:

#include <iostream> #include <list> #include <functional>  class Thing {     private:         int id;         int value = 0;         static int nextId;     public:         Thing() { this->id = Thing::nextId++; };         int getId() const { return this->id; };         int getValue() const { return this->value; };         void add(int n) { this->value += n; }; }; int Thing::nextId = 1;  class Storage {     private:         std::list<std::reference_wrapper<Thing>> list;     public:         void add(Thing& thing) {             this->list.push_back(thing);         }         Thing& findById(int id) const {             for (std::list<std::reference_wrapper<Thing>>::const_iterator it = this->list.begin(); it != this->list.end(); ++it) {                 if (it->get().getId() == id) return *it;             }             std::cout << "Not found!!/n";             exit(1);         } }; 

I started with a simple std::list<Thing>, but then everything is copied around on insertion and retrieval, and I didn't want this because if I get a copy, altering it does not reflect on the original objects anymore. When looking for a solution to that, I found about std::reference_wrapper on this SO question, but now I have another problem.

Now to the code that uses them:

void temp(Storage& storage) {     storage.findById(2).add(1);     Thing t4; t4.add(50);     storage.add(t4);     std::cout << storage.findById(4).getValue() << "/n"; }  void run() {     Thing t1; t1.add(10);     Thing t2; t2.add(100);     Thing t3; t3.add(1000);      Storage storage;     storage.add(t3);     storage.add(t1);     storage.add(t2);      temp(storage);      t2.add(10000);      std::cout << storage.findById(2).getValue() << "/n";     std::cout << storage.findById(4).getValue() << "/n"; } 

My main() simply calls run(). The output I get is:

50 10101 Not found!! 

Although I was looking for:

50 10101 50 

Question

Looks like the locally declared object t4 ceases to exist when the function returns, which makes sense. I could prevent this by dynamically allocating it, using new, but then I didn't want to manage memory manually...

How can I fix the code without removing the temp() function and without having to manage memory manually?

If I just use a std::list<Thing> as some suggested, surely the problem with t4 and temp will cease to exist, but another problem will arise: the code won't print 10101 anymore, for example. If I keep copying stuff around, I won't be able to alter the state of a stored object.

 


Who is the owner of the Thing in the Storage?

Your actual problem is ownership. Currently, your Storage does not really contain the Things but instead it is left to the user of the Storage to manage the lifetime of the objects you put inside it. This is very much against the philosophy of std containers. All standard C++ containers own the objects you put in them and the container manages their lifetime (eg you simply call v.resize(v.size()-2) on a vector and the last two elements get destroyed).

Why references?

You already found a way to make the container not own the actual objects (by using a reference_wrapper), but there is no reason to do so. Of a class called Storage I would expect it to hold objects not just references. Moreover, this opens the door to lots of nasty problems, including undefined behaviour. For example here:

void temp(Storage& storage) {     storage.findById(2).add(1);     Thing t4; t4.add(50);     storage.add(t4);     std::cout << storage.findById(4).getValue() << "/n"; } 

you store a reference to t4 in the storage. The thing is: t4s lifetime is only till the end of that function and you end up with a dangling reference. You can store such a reference, but it isnt that usefull because you are basically not allowed to do anything with it.

Aren't references a cool thing?

Currently you can push t1, modify it, and then observe that changes on the thingy in Storage, this might be fine if you want to mimic Java, but in c++ we are used to containers making a copy when you push something (there are also methods to create the elements in place, in case you worry about some useless temporaries). And yes, of course, if you really want you can make a standard container also hold references, but lets make a small detour...

Who collects all that garbage?

Maybe it helps to consider that Java is garbage-collected while C++ has destructors. In Java you are used to references floating around till the garbage collector kicks in. In C++ you have to be super aware of the lifetime of your objects. This may sound bad, but acutally it turns out to be extremely usefull to have full control over the lifetime of objects.

Garbage? What garbage?

In modern C++ you shouldnt worry to forget a delete, but rather appreciate the advantages of having RAII. Acquiring resources on initialzation and knowing when a destructor gets called allows to get automatic resource management for basically any kind of resource, something a garbage collector can only dream of (think of files, database connections, etc.).

"How can I fix the code without removing the temp() function and without having to manage memory manually?"

A trick that helped me a lot is this: Whenever I find myself thinking I need to manage a resource manually I stop and ask "Can't someone else do the dirty stuff?". It is really extremely rare that I cannot find a standard container that does exactly what I need out of the box. In your case, just let the std::list do the "dirty" work.

Can't be C++ if there is no template, right?

I would actually suggest you to make Storage a template, along the line of:

template <typename T> class Storage { private:     std::list<T> list; //.... 

Then

Storage<Thing> thing_storage; Storage<int> int_storage; 

are Storages containing Things and ints, respectively. In that way, if you ever feel like exprimenting with references or pointers you could still instantiate a Storage<reference_wrapper<int>>.

Did I miss something?...maybe references?

I won't be able to alter the state of a stored object

Given that the container owns the object you would rather let the user take a reference to the object in the container. For example with a vector that would be

auto t = std::vector<int>(10,0);  // 10 element initialized to 0 auto& first_element = t[0];       // reference to first element first_element = 5;                // first_element is an alias for t[0] std::cout << t[0];                // i dont want to spoil the fun part 

To make this work with your Storage you just have to make findById return a reference. As a demo:

struct foo {     private:          int data;     public:         int& get_ref() { return data;}         const int& get_ref() const { return data;} };  auto x = foo(); x.get_ref = 12; 

TL;DR

How to avoid manual resource managment? Let someone else do it for you and call it automatic resource management :P

Comment

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen: