Values of objects in vector reset when trying to get the value, although it was incremented

So as you can see down below, I try increment the x value of the bird by 1. But the value gets reset every time I call the getX() function.

#include <iostream>
#include <vector>

class Bird {
public:
    Bird(double inputX, double inputY) : x(inputX), y(inputY) {

    }

    void tick() {
        x++;
    }
    
    double getX() {
        return x;
    }
    double getY() {
        return y;
    }

private:
    double x, y;
};

class Test {
public:
    void addToVector(double x, double y) {
        Bird bird(x, y);

        birds.push_back(bird);
    }

    std::vector<Bird> getVector() {
        return birds;
    }

private:
    std::vector<Bird> birds;
};

int main(int argc, char* agrs[]) {

    Test test;

    for (int i = 0; i < 5; i++) {
        test.addToVector(0, 0);
    }

    for (int i = 0; i < test.getVector().size(); i++) {
        std::cout << "Index: " << i << " x: " << test.getVector()[i].getX() << " y: " << test.getVector()[i].getY() << std::endl;
    }
    
    while (true) {
        for (int i = 0; i < test.getVector().size(); i++) {
            test.getVector()[i].tick();
        }

        for (int i = 0; i < test.getVector().size(); i++) {
            std::cout << "Index: " << i << " x: " << test.getVector()[i].getX() << " y: " << test.getVector()[i].getY() << std::endl;
        }
    }

}

So I know, that the problem is that I create the bird in the function and "copy" it to the vector. But the bird goes out of scope so I get a memory leak. I tried using unique pointers, but I couldn’t declare the bird with a name because of that. I also don’t know how to prevent that the destruction of the bird

>Solution :

So I know, that the problem is that I create the bird in the function and "copy" it to the vector

No. That’s not a big problem. You could create the bird directly in the vector rather than first creating one then then copy it in the vector. Though, making the copy is not the reason for the output you get, its just a copy that could be avoided.

But the bird goes out of scope so I get a memory leak.

No. There is also no memory leak in your code.

I tried using unique pointers, …

Hm, ok. Not sure what you did, or what was the problem with that code. Anyhow, the actual problem is…

Your getVector returns a copy of the member. Modifying that copy has no effect on the member. The easy fix to get expected output is to change the getter to return a reference:

std::vector<Bird>& getVector() {
    return birds;
}

However, now the question arises why birds is private in the first place. Once you return a non-const referene to the outside of the class the caller can use this reference to do what they like, just as if the member was public.

Rather than designing your classes as data containers with getters and setters to set and get their members you should design the classes according to their behavior (unless the aim is to implement a plain data container of course). Here, rather than returning the whole vector to the caller you could let the caller specify which bird they want to tick and provide a mehtod to print contents of the vector. Something along the line of (not tested, and not changing Bird other than removing one member):

class Bird {
public:
    Bird(double inputX) : x(inputX) {}
    void tick() {  x++; }
    double getX() const { return x; }  // should (/must) be const !
private:
    double x;
};

class Birdies {
public:
    void addToVector(double x) {
        birds.push_back(x);  // still creates a tempory that is copied, look at emplace_back
    }
    void makeTick(size_t index) {
        birds[index].tick();
    }
    void print() const {
        for (const auto& b : birds) std::cout << b.getX() << "\n";
    }
private:
    std::vector<Bird> birds;
};

Leave a Reply