Losing some data after leaving function

Question

I've got a class which contains these vectors:

class MainMenu {

private:
     std::vector <Car> vehicles;
     std::vector <Instructor> instructors;
     std::vector <Trainee> trainees;
     std::vector <Lesson> lessons;
public:
     std::vector<Car>& getVehicles();
     std::vector<Instructor>& getInstructors();
     std::vector<Trainee>& getTrainees();
     std::vector<Lesson>& getLesson();
}

//-------------------------------------------------

class Car : public Vehicle {

public:
    static MARKA convertStringToMarka(std::string name);
    Car(std::string numberPlate, MARKA marka, bool roadWorthy);
    Car() {};
    Car(int id,std::string numberPlate, MARKA marka, bool roadWorthy);

};

And somewhere in the code i perform such function:

Car *car = mainMenu.searchForCarById(idOfVehicle);

Car* MainMenu::searchForCarById(int id) {
     for (Car elem : vehicles) {
         if (elem.getIdInSystem() == id) {
             return &elem;
         }
    }
}

While debugging i perform this code step by step, and this simply function search for car's correctly but whenever I return from funtion, every variable which was string dissapear. For example in name variable I recieve "". I think it is important to mention that only strings dissapear! int stays as it was. What am i doing wrong?

You are returning an address of a local variable, which is is erased after you live the function. This is undefined behavior. In your case, you should make your `elem` a reference - this would work fine, and have an added benefit of removing unnecessary copy. — SergeyA, Mar 12 '18 at 18:48
You also have a "not all code paths return a value" error in `searchForCarById()`. Next time please provide a proper [mcve]. — TypeIA, Mar 12 '18 at 18:49
Unrelated, but why do you have private class members, and then provide public accessors that return mutable references? Might as well make those class members public to begin with. — IInspectable, Mar 12 '18 at 18:54

Cory Kramer · Answer 1 · 2018-03-12T18:49:39.430

5

Change this line

for (Car elem : vehicles)

to

for (Car& elem : vehicles)

otherwise you are saving a pointer to a local variable that falls out of scope, therefore working against a dangling pointer

edited Mar 12 '18 at 18:49

answered Mar 12 '18 at 18:48

Cory Kramer

114,268
16
167
218

score 4 · Answer 2 · answered Mar 12 '18 at 18:48

4

for (Car elem : vehicles)

elem is a copy of the actual object contained in vehicles. You then return a pointer to this local object which is destroyed, leaving it dangling. The fix is to use Car& instead.

answered Mar 12 '18 at 18:48

Hatted Rooster

35,759
6
62
122

score 4 · Answer 3 · answered Mar 12 '18 at 18:48

When you loop through your vehicles, the following line:

for (Car elem : vehicles)

makes a copy of each element and stores it in elem.

Instead, try getting the element by reference:

for (Car& elem : vehicles)

Then, you can actually get a pointer. However, This pointer might become invalid as soon as you modify your array. So it's still not optimal.

Losing some data after leaving function

3 Answers3