0

I have a task to create an object Stos which would feature a heap of objects Obiekt, to which I could add things as I please.

In order to make the program better support dynamic arrays I decided to use a Vector. The whole implementation seems to run perfectly, the returned value is completely off. Here is an example with code:

class Obiekt {
private:
    int id;

public:
    Obiekt::Obiekt(int i) {
        id = i;
    }

    void Obiekt::display() {
        cout << "This object has id of: " << id << endl;
    }
};

class Stos {
private:
    vector < Obiekt* > stos;
public:
    Stos::Stos(Obiekt n) {
        add(n);
    }

    void Stos::add(Obiekt n) {
        stos.push_back(&n);
    }

    void Stos::display() {
        cout << endl << "===HEAP DISPLAY===" << endl;
        for (int i = 0; i < stos.size(); i++) {
            stos[i]->display();
        }
    }
};

void Zad1()
{
    Obiekt obj1(5);
    Obiekt obj2(23);

    Stos s1(obj1);
    s1.add(obj2);
    s1.display();

    getchar();
}

And the outcome being:

===HEAP DISPLAY===

This object has id of: -858993460

This object has id of:9805925

I'm not a cpp expert, and believe the issue is related to the stos.push_back(&n) portion, but I can't catch the moment the id gets so distorted.

It's probably a noob question, so sorry for that on start.

Any help would be amazing.

Community
  • 1
  • 1
aln447
  • 981
  • 2
  • 15
  • 44
  • @O'Neil Worked!! Thanks. Could you post this as an answer? – aln447 Dec 06 '17 at 23:18
  • Answer posted :) – O'Neil Dec 06 '17 at 23:23
  • The simplest solution is to make the vector be `vector < Obiekt > stos;` – M.M Dec 06 '17 at 23:27
  • 1
    Since it appears you're developing for windows/VS, I recommend you learn to recognize the value "-858993460" (0xCCCCCCCC). This is one of several special values with which Visual Studio fills newly allocated memory when you're building in Debug mode. If you see this value, it's a pretty good indicator you have a memory error going on. https://www.softwareverify.com/memory-bit-patterns.php – Shirik Dec 06 '17 at 23:36

3 Answers3

3

The issue with your code as O'Neil correctly explained is that you're adding the pointer to a copy of the Obiekt object. So basically, you create your object in main, and pass it to the constructor and the .add function in Stos. You then add the pointer to the vector. When the function finishes, the copy that was passed is destroyed and the pointer in your vector is dangling. There are two ways to fix this:

1 Pass by reference

This is very simple, basically you just add an ampersand to your function parameters. For instance:

void Stos::add(Obiekt &n) {
    stos.push_back(&n);
}

This will ensure that the object isn't destroyed at the end of the function

2 Don't use pointers

Another way of getting your problem to work is to avoid using pointers at all. Your vector will actually copy the contents of the Obiekt object into it. For example:

vector < Obiekt > stos; // notice how we define it without the pointer type

...

void Stos::add(Obiekt n) {
    stos.push_back(n); // Creates copy which will then contain the correct value
}
Community
  • 1
  • 1
Adam Snaider
  • 188
  • 1
  • 8
1

The parameters Obiekt n in

Stos::Stos(Obiekt n) {
    add(n);
}

void Stos::add(Obiekt n) {
    stos.push_back(&n);
}

are temporary copies destroyed immediatly after each call.

You have to use a reference Obiekt & n instead, or better: by pointer Obiekt * n.

O'Neil
  • 3,790
  • 4
  • 16
  • 30
  • 3
    It is conventional that if a parameter is passed by reference then its address should not be stored beyond the call, so this is not good style – M.M Dec 06 '17 at 23:27
0

I'm reluctant to assert that the objects exist at the time display is called.

Problem(s)

According to GCC's implementation they don't. They fall out of scope and are immediately destructed. Give "Obiekt" a non-trivial destructor and this behavior becomes obvious:

~Obiekt(){std::cout << "Bye from: " << it << std::endl;}

Secondarily, note that you shouldn't specify the class membership for functions defined within the class itself (no class_name::function_name(parameters), just function_name(parameters) )

Possible Fix

You (might) want to changing "Stos" such that:

Stos(Obiekt &n) {add(n);}
void add(Obiekt &n) {stos.push_back(&n);}
C3H8
  • 46
  • 4