static function to call member function on all objects?

[cpengr]

Remember objects in stl containers are copied in and copied out.

Ah, I had forgotten.

You store the this of a temporary yet this gets copied into the vector. MyClass relies on a default copy constructor, so your set becomes out-of-sync with the real this pointers.

Provide a copy constructor that updates the set correctly.

Are you saying that the objects are being copied, or that the pointers are being copied?
1.) If the objects are being copied, why? The container only holds pointers to the objects.
2.) Or are the pointers being copied, and then the objects are being moved around in memory, such that the copies of the pointers no longer point to the objects?

Well, you're making my point.

What if when optimizations are turned on (which you want turned on for release versions of your code), the output is different? What do you do then?

Hopefully you are observing this behaviour only for educational purposes. If you are truly planning on writing a program based on when objects destroy themselves, then you're going about this the wrong way.

Regards,

Paul McKenzie

Thanks for the warning...

It was never my intent to rely on some "compiler quirk" to get my program working. I'm just not clear on what's happening here, or what assumptions I have that are causing me to expect something else to occur...

My goal is to have a class which contains a static function which, when called, calls a member function on all instances of the class. My purpose for the code shown above was just to be a proof of concept, that I could track all the existing instances of the class--I probably would have added the static function next. However, it seems some instances are being destroyed before I expect, or there are copies taking place that I'm unaware of, or the pointers in the set are being invalidated, or...?

So, if I am going about this the wrong way, can you suggest a right way?

Thanks.

[Paul McKenzie]

My purpose for the code shown above was just to be a proof of concept, that I could track all the existing instances of the class--

Copying an object should be considered a "base" or "atomic" operation in C++. You do not have total control on when copying takes place or not, unless you make the copy constructor (which you didn't consider) private (then no copying would be allowed at compile-time).

However, it seems some instances are being destroyed before I expect, or there are copies taking place that I'm unaware of,

Again, you never write a C++ program that depends on when or where copies are done.

The compiler can make copies automatically if you pass by value, return by value, use conversion operators, etc. In this case, none of those copies were generated by you explicitly. Similarly, the compiler is free to remove copying and instances of objects due to optimizations (Return Value Optimization is one such optmization technique).

Trying to track instances exactly just doesn't work, unless you are writing some sort of program that tests compilers and how they behave, or some similar utility. The main reason why it isn't a good idea to write a program in the way you have done is that your program will experience different behaviour depending on compiler optimizations, options, etc. You upgrade the compiler to a later version, and yet again, the same program you wrote may have different behaviour.

So, if I am going about this the wrong way, can you suggest a right way?

1) Store pointers in the vector.

2) Turn off the copying completely by making the copy constructor and assignment operator private and unimplemented.

3) Add a static function to the class to explicitly create objects so that the compiler has no chance of doing so itself. Then the user just can't create objects on the fly by just declaring them -- you have total control over how the objects are created, i.e. the factory pattern.

Regards,

Paul McKenzie

[cpengr]

Thanks for the detailed answer (including the part I'm not quoting here).

1) Store pointers in the vector.

2) Turn off the copying completely by making the copy constructor and assignment operator private and unimplemented.

3) Add a static function to the class to explicitly create objects so that the compiler has no chance of doing so itself. Then the user just can't create objects on the fly by just declaring them -- you have total control over how the objects are created, i.e. the factory pattern.

Factory pattern, hrm. I did a little reading, and...this is starting to seem like more trouble than it's worth. I was trying to make things easier for the end-user of this class (just me, so far). I think since I will have a known, finite amount of instances, it will be easier to just make an array of "MyClass"es, and make a (static or non-member) function that calls the member function on all of them. I think this will be easier to maintain, whether by me or someone else. I'm interested to know if you think otherwise.

I'm an EE, working on my master's in CpE...some of the more "esoteric" aspects of s/w design have eluded me thus far...

[Russco]

What I was explaining was in the code you stuff default constructed objects into a vector.

First the default object gets created. At creation the this pointer is stored in the set. This 'this pointer' points to a temporary object.

Second this default constructed object is passed as a parameter to vector:ush_back which COPIES that object into the vector. Where does this copy come from? It comes from the copy constructor that because you neither provided one nor disallowed it, the compiler nicely generated for you. However that implicit copy constructor does not cause the set to be updated to reflect the new object in existence. Its why you get all those could not find pointer in set messages.

Whilst Paul is telling you never to write code that depends on destruction order that you cannot manage, I'm pointing out that you haven't even correctly kept a tab on 'live objects'.

[Joeman]

all you need to do is implement the

MyClass(const MyClass& p); // copy constructor
MyClass& operator =( const MyClass& cpy ); //(optional) assignment

in short make these add the this pointer to the container.
or you can do what paul said.

[cpengr]

What I was explaining was in the code you stuff default constructed objects into a vector.

First the default object gets created. At creation the this pointer is stored in the set. This 'this pointer' points to a temporary object.

Second this default constructed object is passed as a parameter to vector:ush_back which COPIES that object into the vector. Where does this copy come from? It comes from the copy constructor that because you neither provided one nor disallowed it, the compiler nicely generated for you. However that implicit copy constructor does not cause the set to be updated to reflect the new object in existence. Its why you get all those could not find pointer in set messages.

Whilst Paul is telling you never to write code that depends on destruction order that you cannot manage, I'm pointing out that you haven't even correctly kept a tab on 'live objects'.

Ah-hah! That helps clarify what's happening, thank you.

all you need to do is implement the

MyClass(const MyClass& p); // copy constructor
MyClass& operator =( const MyClass& cpy ); //(optional) assignment

in short make these add the this pointer to the container.
or you can do what paul said.

Okay, so by adding a copy constructor, and disallowing assignment (and subsequently remming assignments in doit())...

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

#include <vcl.h>
#include <set>
#include <vector>
#include <iostream>
#pragma hdrstop

using std::cout;
using std::set;
using std::vector;

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

class MyClass {
    public:
        MyClass();
        ~MyClass();
        MyClass(const MyClass& ref);  // copy constructor
        void Print();
    private:
        static int count;
        static set<MyClass*> setOfPtrs;
        int num;
        MyClass& operator=(const MyClass& cpy);  // disallow assignment
};

set<MyClass*> MyClass::setOfPtrs;
int MyClass::count = 0;

MyClass::MyClass() {
    num = ++count;
    setOfPtrs.insert(this);
    cout << "creating "   << num   << "; "
         << setOfPtrs.size() << " objects exist.\n";
}

MyClass::~MyClass() {
    if (setOfPtrs.find(this) == setOfPtrs.end()) {
        cout << "\t(destructor " << num
             << " could not find ptr!!)\n";
    } else {
        cout << "destroying " << num   << "; ";
        setOfPtrs.erase(setOfPtrs.find(this));
        cout << setOfPtrs.size() << " objects exist.\n";
    }
}

MyClass::MyClass(const MyClass& ref) {  // copy constructor
    num = ++count;
    setOfPtrs.insert(this);
    cout << "creating "   << num   << ", copy of " << ref.num << "; "
         << setOfPtrs.size() << " objects exist.\n";
}

void MyClass::Print() {
    cout << "  #" << num << " exists\n";
}

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

void doit() {
    cout << "doit:\n";
    MyClass m1, m2, m3;
    m1.Print();
    m2.Print();
    m3.Print();
    //m2 = MyClass();
    //m2.Print();
    MyClass m4;
    m4.Print();
    //m1 = m4;
    //m1.Print();
}

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

void doitVecNoRef() {
    cout << "doitVecNoRef:\n";
    vector<MyClass> myVec;
    for (int i=0; i<3; i++) {
        myVec.push_back(MyClass());
    }

    for (vector<MyClass>::iterator it=myVec.begin(); it != myVec.end(); it++) {
        it->Print();
    }
}

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

void doitVecOfPtrs() {
    cout << "doitVecOfPtrs:\n";
    vector<MyClass*> myVec;
    for (int i=0; i<3; i++) {
        myVec.push_back(&MyClass());
    }

    for (vector<MyClass*>::iterator it=myVec.begin(); it != myVec.end(); it++) {
        (*it)->Print();
    }
}

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

#pragma argsused
int main(int argc, char* argv[])
{
    doit();
    cout << "\n\n";
    doitVecNoRef();
    cout << "\n\n";
    doitVecOfPtrs();
    cout << "\n\n";
    return 0;
}
//---------------------------------------------------------------------------

I get

doit:
creating 1; 1 objects exist.
creating 2; 2 objects exist.
creating 3; 3 objects exist.
  #1 exists
  #2 exists
  #3 exists
creating 4; 4 objects exist.
  #4 exists
destroying 4; 3 objects exist.
destroying 3; 2 objects exist.
destroying 2; 1 objects exist.
destroying 1; 0 objects exist.


doitVecNoRef:
creating 5; 1 objects exist.
creating 6, copy of 5; 2 objects exist.
destroying 5; 1 objects exist.
creating 7; 2 objects exist.
creating 8, copy of 6; 3 objects exist.
creating 9, copy of 7; 4 objects exist.
destroying 6; 3 objects exist.
destroying 7; 2 objects exist.
creating 10; 3 objects exist.
creating 11, copy of 8; 4 objects exist.
creating 12, copy of 9; 5 objects exist.
creating 13, copy of 10; 6 objects exist.
destroying 8; 5 objects exist.
destroying 9; 4 objects exist.
destroying 10; 3 objects exist.
  #11 exists
  #12 exists
  #13 exists
destroying 11; 2 objects exist.
destroying 12; 1 objects exist.
destroying 13; 0 objects exist.


doitVecOfPtrs:
creating 14; 1 objects exist.
destroying 14; 0 objects exist.
creating 15; 1 objects exist.
destroying 15; 0 objects exist.
creating 16; 1 objects exist.
destroying 16; 0 objects exist.
  #16 exists
  #16 exists
  #16 exists

In this case, that's 3x as many created vs. how many are needed, for use in the vector. I think they are created when the vector capacity is enlarged, and then by the MyClass() inside the push_back, then copied by the insertion (push_back). Using vector::reserve(), I was able to cut it down to just the copy for insertion.

Some questions remain:

1. Is it possible to populate a container of MyClass objects without any copies being performed, and without having individual variable names for each? (i.e. I don't want to have

MyClass m1;
MyClass m2;
//...
myVec.push_back(m1);
myVec.push_back(m2);
//... )

2. ...using a non-default constructor?


Thanks again, all.

[Joeman]

1. Is it possible to populate a container of MyClass objects without any copies being performed, and without having individual variable names for each? (i.e. I don't want to have

you mean like
vector<MyClass*> myVec;
myVec.push_back( new MyClass() );

? perhaps you want to use the list container as well.

make sure you delete them too

[cpengr]

you mean like
vector<MyClass*> myVec;
myVec.push_back( new MyClass() );

make sure you delete them too

That crossed my mind. Ironically, part of what drew me to STL containers was not having to manage memory...oh well, guess I can't have my cake & eat it, too...but I do like to know what kinds of cake are available.

? perhaps you want to use the list container as well.

Probably not in this situation, but thanks for the reminder.

[laserlight]

That crossed my mind. Ironically, part of what drew me to STL containers was not having to manage memory...

Well, you do not have the manage the memory of the dynamic array of pointers even though you have to manage the memory for the object associated with each pointer.

oh well, guess I can't have my cake & eat it, too...but I do like to know what kinds of cake are available.

If a TR1 implementation is available, you can use a std::vector<std::tr1::shared_ptr<MyClass> > instead. If Boost is available you could use a boost::ptr_vector<MyClass>. You could always wrap the vector with another class whose sole duty is to perform memory management for the object associated with each pointer in the vector.

[cpengr]

Ironically, part of what drew me to STL containers was not having to manage memory...

Well, you do not have the manage the memory of the dynamic array of pointers even though you have to manage the memory for the object associated with each pointer.

Good point. I think maybe I will have my "main class" create the collection in its constructor, and destroy it in its destructor...such that the objects in the collection will persist through the duration of the program and be reused, vs. being created and destroyed every time the user clicks "start".

If a TR1 implementation is available, you can use a std::vector<std::tr1::shared_ptr<MyClass> > instead. If Boost is available you could use a boost::ptr_vector<MyClass>. You could always wrap the vector with another class whose sole duty is to perform memory management for the object associated with each pointer in the vector.

I'm using Borland C++ Builder v6. I don't think it's worth the trouble in this case. Thanks for the pointer, though. (No pun intended.)

[cpengr]

No extra copies...

void doitVecNew() {
    cout << "doitVecNew:\n";
    cout << "  create myVec<MyClass*>...\n";
    vector<MyClass*> myVec;
    cout << "  populate myVec (push_back(new MyClass())):\n";
    for (int i=0; i<3; i++) {
        myVec.push_back(new MyClass());
    }
    cout << "  delete members of myVec:\n";
    for (int i=0; i<3; i++) {
        delete myVec[i];
    }
    cout << "  finished deletions, exiting doitVecNew...\n";
}

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

#pragma argsused
int main(int argc, char* argv[])
{
    doitVecNew();
    return 0;
}

Output:

doitVecNew:
  create myVec<MyClass*>...
  populate myVec (push_back(new MyClass())):
        creating 25; 1 objects exist.
        creating 26; 2 objects exist.
        creating 27; 3 objects exist.
  delete members of myVec:
        destroying 25; 2 objects exist.
        destroying 26; 1 objects exist.
        destroying 27; 0 objects exist.
  finished deletions, exiting doitVecNew...

[cpengr]

This seems appropriate:

[In] C, you shoot yourself in the foot.

[In] C++, you accidentally create a dozen instances of yourself and shoot them all in the foot. Providing emergency medical assistance is impossible since you can't tell which are bitwise copies and which are just pointing at others and saying, "That's me, over there."

Excerpted from http://www.softpanorama.org/Lang/Cpp_rama/humor.shtml.