Iterator and handle confusion

[James Gregory]

I was having some issues with a program involving iterators and handles,
and decided to try and make a short test program to try to work out where
my problem was. However, my test program went wrong long before it
resembled anything like my actual program.

Here is the short test program:

#include <iostream>
#include <list>

using std::cout;
using std::endl;
using std::list;

int globalCounter = 0;

class MyClass
{
public:
MyClass() {classCounter = globalCounter++;}

int classCounter;
};

class MyHandle
{
public:
MyHandle() {myPointer = new MyClass; handleCounter = myPointer->classCounter;}
~MyHandle() {delete myPointer;}
int GetCounter() {return myPointer->classCounter;}

int handleCounter;

private:
MyClass* myPointer;
};

int main()
{
list<MyHandle> myList;

for (int i = 0; i != 10; ++i)
myList.push_back(MyHandle());

for (list<MyHandle>::iterator iter = myList.begin(); iter != myList.end(); ++iter)
cout << iter->handleCounter << " " << iter->GetCounter() << endl;

return 0;
}

The output I get is:

0 0
1 0
2 0
3 0
4 0
5 0
6 0
7 0
8 0
9 0

But I was expecting:

0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9

Where have I gone wrong?

Thanks,

James

[Cy Edmunds]

"James Gregory" <ja****@f2s.com> wrote in message
news:pa****************************@f2s.com...

I was having some issues with a program involving iterators and handles,
and decided to try and make a short test program to try to work out where
my problem was. However, my test program went wrong long before it
resembled anything like my actual program.

Here is the short test program:

#include <iostream>
#include <list>

using std::cout;
using std::endl;
using std::list;

int globalCounter = 0;

class MyClass
{
public:
MyClass() {classCounter = globalCounter++;}

int classCounter;
};

class MyHandle
{
public:
MyHandle() {myPointer = new MyClass; handleCounter = myPointer->classCounter;} ~MyHandle() {delete myPointer;}
The rule of 3: if a class has a destructor, a copy constructor, or an
assignment operator, it probably needs all three. Your code actually crashes
on my compiler because you disobeyed this rule. Add the following:

MyHandle(const MyHandle &other) {myPointer = new MyClass; handleCounter =
other.handleCounter;}
MyHandle &operator = (const MyHandle &other)
{
if (&other != this)
{
delete myPointer;
myPointer = new MyClass;
handleCounter = other.handleCounter;
}
return *this;
}
int GetCounter() {return myPointer->classCounter;}

int handleCounter;

private:
MyClass* myPointer;
};

int main()
{
list<MyHandle> myList;

for (int i = 0; i != 10; ++i)
myList.push_back(MyHandle());

for (list<MyHandle>::iterator iter = myList.begin(); iter != myList.end(); ++iter) cout << iter->handleCounter << " " << iter->GetCounter() << endl;

return 0;
}

The output I get is:

0 0
1 0
2 0
3 0
4 0
5 0
6 0
7 0
8 0
9 0

But I was expecting:

0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9

Where have I gone wrong?

Thanks,

James

The corrected output is:

0 1
2 3
4 5
6 7
8 9
10 11
12 13
14 15
16 17
18 19

which I believe is correct when you account for the fact that a copy is made
during the push_back.

--
Cy
http://home.rochester.rr.com/cyhome/

[John Harrison]

"James Gregory" <ja****@f2s.com> wrote in message
news:pa****************************@f2s.com...

I was having some issues with a program involving iterators and handles,
and decided to try and make a short test program to try to work out where
my problem was. However, my test program went wrong long before it
resembled anything like my actual program.
Good strategy.

Here is the short test program:

#include <iostream>
#include <list>

using std::cout;
using std::endl;
using std::list;

int globalCounter = 0;

class MyClass
{
public:
MyClass() {classCounter = globalCounter++;}

int classCounter;
};

class MyHandle
{
public:
MyHandle() {myPointer = new MyClass; handleCounter = myPointer->classCounter;} ~MyHandle() {delete myPointer;}
int GetCounter() {return myPointer->classCounter;}

int handleCounter;

private:
MyClass* myPointer;
};

int main()
{
list<MyHandle> myList;

for (int i = 0; i != 10; ++i)
myList.push_back(MyHandle());

for (list<MyHandle>::iterator iter = myList.begin(); iter != myList.end(); ++iter) cout << iter->handleCounter << " " << iter->GetCounter() << endl;

return 0;
}

The output I get is:

0 0
1 0
2 0
3 0
4 0
5 0
6 0
7 0
8 0
9 0

But I was expecting:

0 0
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 8
9 9

Where have I gone wrong?

Thanks,

James

What you've done wrong is to ignore the issue of copying. EVERY TIME you
create a class, you should ask yourself the question 'what happens if an
object of this class is copied?'. If you look at your MyHandle class, then
if a MyHandle object is copied it will get a copy of the myPointer member
variable. Now what happens if the original MyHandle object is deleted? Your
copy will still be holding a pointer to a now deleted MyClass object. I
think you were lucky to get the program to run at all, it crashed when I ran
it with my compiler.

What is actually happening is this program is that you create a MyHandle
object, then the std::list takes a copy of the object, the original is
destroyed, deleting the myPointer pointer in its destructor, so you have an
object on a list that contains a pointer to a now deleted object. Hence (in
your case) the zeros.

Try this

class MyHandle
{
// code as before

/* copy constructor, called when the object is copied */
MyHandle(const MyHandle& x)
{
myPointer = new MyClass;
handleCounter = myPointer->classCounter;
}

/* assignment operator, called when the object is assigned (doesn't
actually happen in your program) */
MyHandle& operator=(const MyHandle& x)
{
return *this;
}

};

Of course the copy semantics I have chosen might not be the ones you want,
but at least I defined some sort of sensible copying behaviour whereas
you've just ignored the issue.

Any decent C++ book will cover this issue, you might also want to look up
the topic of reference counting.

John

[James Gregory]

On Sat, 07 Feb 2004 20:13:13 +0000, James Gregory wrote:

<snip>

Many thanks for the replies, I forgot about how important dealing with
copying is.

However, my "real" program does actually have a copy constructor and
reference counting in the handle, which I added after "reading a good C++
book", so I've still got to work a bit more to find my main problem.

James

[Vyacheslav Lanovets]

Hello, James!
You wrote on Sat, 07 Feb 2004 20:13:13 +0000:

JG> I was having some issues with a program involving iterators and

[Sorry, skipped]

JG> class MyHandle
JG> {
JG> public:
JG> MyHandle() {myPointer = new MyClass; handleCounter =
JG> myPointer->classCounter;} ~MyHandle() {delete myPointer;}
JG> int GetCounter() {return myPointer->classCounter;}

[Sorry, skipped]

JG> for (int i = 0; i != 10; ++i)
JG> myList.push_back(MyHandle());

Here it creates an instance of MyHandle
Then copies its contents into the list using bitwise copy (no user defined
operator= was provided)
Then destructor of MyHandle is called, invalidating the pointer

Booom, you've got a whole list of garbage :)

Vyacheslav Lanovets