-1

I'm trying to make this code work, but the object keep getting destroyed... I've found that it has to do with the object being copied to the vector, but can't find any way to prevent it...

#include <iostream>
#include <string>
#include <vector>

using namespace std;

class Obje
{
private:
    static int instances;
    int id;

public:
    static int getInstances();
    void getId();
    virtual void myClass();

    Obje(int auxId);
    ~Obje();
};

int Obje::instances = 0;

int Obje::getInstances()
{
    return instances;
}

Obje::Obje(int auxId)
{
    this->id = auxId;
    cout << "Obje Created." << endl;
    Obje::instances++;
}

Obje::~Obje()
{
    cout << "Obje Destroyed." << endl;
    Obje::instances--;
}

void Obje::myClass()
{
    cout << "Obje." << endl;
}

void Obje::getId()
{
    cout << this->id << endl;
}

int main()
{
    vector <Obje> list;
    Obje *a = new Obje(59565);
    list.push_back(*a);
    Obje *b = new Obje(15485);
    list.push_back(*b);

    for(vector<Obje>::iterator it = list.begin(); it != list.end(); ++it)
    {
        it->getId();
    }

    return 0;
}

It Generates this output:

Obje Created.
Obje Created.
Obje Destroyed.
59565
15485
Obje Destroyed.
Obje Destroyed.

What does it mean the T(const T& new); i've saw as fix for this?

HrejWaltz
  • 45
  • 1
  • 2
  • `T(const T&obj);` is the _copy constructor_. Look it up. – 1201ProgramAlarm Dec 14 '15 at 05:45
  • Firstly, you need to read [this comment](http://stackoverflow.com/questions/3428750/memory-leak-with-stdstring-when-using-stdliststdstring#comment3570156_3428750), next read [this question](http://stackoverflow.com/questions/6500313/why-should-c-programmers-minimize-use-of-new) to understand how to use memory in C++. This is C++ not Java. – Danh Dec 14 '15 at 05:45
  • You don't need any `new` expressions here. They do nothing but create nemory leaks for you. C++ is not Java. – n. 'pronouns' m. Dec 14 '15 at 05:47
  • Not using `new` i got 5 calls to the destructor... How do i implement this copy constructor? – HrejWaltz Dec 14 '15 at 05:50
  • Of course destructor must be executed eventually! What's wrong? –  Dec 14 '15 at 05:53
  • When i call getInstances(), it is returning the wrong number bcz of all this calls of the destructor... – HrejWaltz Dec 14 '15 at 05:55
  • So, what is the *right* number? How do you define "right number"? –  Dec 14 '15 at 05:57
  • I've created 2 objects.... but when i call Obje::getInstances(), it returns -1... the instance count is incremented in the constructor and decremented in the destructor... – HrejWaltz Dec 14 '15 at 05:58
  • That is becuse the instance count is not increamented in *every* constructor. And don't count instances. –  Dec 14 '15 at 06:00
  • @NickyC is right, `instance` needs to be increased in copy/move constructor, too. – Danh Dec 14 '15 at 06:07

2 Answers2

2

First of all, it is a bad practice to allocate an object in heap without using smart pointers and forgetting to delete it. Especially, when you are creating it just to make a copy of it.

list.push_back(*a); creates a copy of *a in vector. To create an item in vector without copying another item, you can do list.emplace_back(/*constructor parameters*/);, which is available from c++11. (see http://en.cppreference.com/w/cpp/container/vector/emplace_back)

So, to make the result behavior match your expectations, you should go

    vector <Obje> vec;
    vec.emplace_back(59565);
    vec.emplace_back(15485);

    for(const auto & item : vec)
    {
        item.getId();
    }

By the way, it is also a quite bad practice to call a vector as a list, as a list is a different container type and reading such code may be confusing a bit. I guess, I am starting being annoying, but it is better to call method getId as showId as now it returns nothing.

V. Kravchenko
  • 1,753
  • 7
  • 11
  • 1
    `emplace_back` is available from C++11, I think it should be put in remark. – Danh Dec 14 '15 at 05:55
  • 1
    @HrejWaltz With most recent Eclipse and GCC, Project->properties->C/C++ Build->Settings->GCC C++ compiler->Dialect->Language Standard Drop-down. – user4581301 Dec 14 '15 at 06:04
  • emplace_back() keep unresolved... Done it and added the flags in c/c++ compiler->miscellaneous... – HrejWaltz Dec 14 '15 at 06:12
0

Regarding the use of heap, new and pointer, see my comment in your question.

Regarding the issue object was destroyed, the vector maintains an internal buffer to store object. When you push_back new object to the vector, if its internal buffer is full, it will (the stuff which will be executed when exception occurs won't be mentioned here.):

  1. allocate new internal buffer which is big enough to store its new data.
  2. move data from old internal buffer to new internal buffer.
  3. destroy old buffer.

Hence, your object will be destroyed and copied to new location in this case, hence copy constructor will make it clearer to you.

P/S: AFAIK, some compilers move its data by memmove or std::move

Danh
  • 5,455
  • 7
  • 26
  • 41