Problems with updating a vector of pointers in a constructor

Question

stackoverflow! For my latest assignment, I came across a problem I can't really wrap my head around.
The idea behind it is really easy, you have a basic class called Dir_or_File, which represents (as the name suggests) a directory or a file. Every one of these has two variables within itself, it has a name which corresponds to its path and entries, which consists of a vector, which keeps track of all the directories or files, which are contained in my directory.

To aid in this, I have made a method called split_after_slash, which converts any string into a vector of strings, which are split after each slash, "o1/o2/o3/" becomes a vector with the entries "o1/", "o2" and "o3". This method has been tested and it works as intended. Just for clarification, as I will be using it later on.

With this in mind, the constructor is causing problems, the idea is, that I give a path (as a string) to my constructor and it will initiate a directory.
For example:

Dir_or_File dir("/home/music/song.mp3");

Should create the Dir_or_File object "dir", which consists of its name, which is "/home/music/song.mp3" and a vector named entries, which would itself contain a Dir_or_File with the name"home/music/song.mp3" and entrieswith a Dir_or_File with the name "music/song.mp3" in it and so on. At the end, you would have your last entry, named "song.mp3", which has an empty list of entries.

My idea is to solve said problem via recursion, which I tried the following way:

Dir_or_File::Dir_or_File(std::string name)
{
  this->name = name;
  std::vector<std::string> full_path = split_after_slash(name);

  if (full_path.size() > 1)
  {
    std::string new_param;
    for (unsigned int i = 1; i < full_path.size(); i++)
    {
        new_param.append(full_path[i]);
    }
    Dir_or_File new_entry(new_param);
    entries.push_back(&new_entry);
  }
}

Setting the nameis self-explainatory, splitting the string should be too. After that, I only need to know if there is more than one string remaining in the path, which means I would be in a folder, which still has a sub-folder, if there is a sub-folder, I will call the constructor with everything but the folder I'm currently in, if there is no sub-folder, I don't need to add any entries to, well entries.
But if I test my code now, I can create a Dir_or_File, which has one entry in entries, but thats it, the entry has no name, but some gibberish and an empty entries vector, what am I doing wrong here?

Dereferencing pointers of objects that no longer exist is undefined behavior -> That's where the gibberish comes from. — Rakete1111, Jul 23 '17 at 18:06
Is this something that is fundamentally wrong with my approach or something that can be fixed? — DerPanda93, Jul 23 '17 at 18:07
Consider de-pointering `entries` and using `entries.emplace_back(new_param)`. `vector`s are best left to manage their own memory. [Docs on emplace_back](http://en.cppreference.com/w/cpp/container/vector/emplace_back) — user4581301, Jul 23 '17 at 18:08
@DerPanda93 Jup, but you can easily fix it. You could also read [this question](https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope), it might give you a little more insight into the problem. — Rakete1111, Jul 23 '17 at 18:08
@Rakete1111 so basically, new_entry only existed during the if-statement, and the pointer I pushed into "entries" is invalid once that if-statement is over? — DerPanda93, Jul 23 '17 at 18:15
@Rabbid76 the type of the vector is preset, as it is an assignment, sadly, I can't change it. — DerPanda93, Jul 23 '17 at 18:17
@Rakete1111 Any chance to prevent that within this method? Might be a dumb question, but I have to do it within this constructor. Recursion seems necessary, but where would i declare it instead? — DerPanda93, Jul 23 '17 at 18:18
@DerPanda93 No, you cannot prevent destruction of variables when they get out of scope. For the alternatives, the answer(s) below help :) — Rakete1111, Jul 23 '17 at 18:20

score 3 · Accepted Answer · answered Jul 23 '17 at 18:18

Is this something that is fundamentally wrong with my approach or something that can be fixed?

Yes. Yes.

The lines

Dir_or_File new_entry(new_param);
entries.push_back(&new_entry);

are the source of your problems. new_entry is a local variable in the function. Storing its address in entries is not good. When the function returns, entries will contain a list of dangling pointers.

You can fix the problem using couple of approaches.

Make entries a vector of objects instead of vector of pointers. Then, you can use
```
 entries.push_back(new_entry);
```

Allocate dynamic memory using new and store the pointer in entries.

Dir_or_File* new_entry = new Dir_of_File(new_param);
entries.push_back(new_entry);

Note that, if you use the second approach, you'll have to write a bunch of booking code to manage pointers. It will be easier to user the first approach, which will also result in less buggy code.

You can store smart pointers in stead of raw pointers in entries but even that requires you to understand how to smart pointers. The first approach is still better, IMO.

Thank you, that did it for me! I am limited to the second approach due to how the assignment was set, but I guess I understand it now! — DerPanda93, Jul 23 '17 at 18:20

Problems with updating a vector of pointers in a constructor

1 Answers1