OSDev.org

Posted: **Wed Feb 01, 2006 5:54 am**

The basic flow at the moment is:

Create the object (not a pointer to it, an actual object)
Call it's Setup() method to do various bits of housekeeping
Push it onto the vector

I think I've identified one other issue with my own copy of the object going out of scope at the end of the method, there are a couple of things in there on the ends of pointers that are freed when it is destroyed, and the pushed copy would still be holding the same pointers. I don't think that is causing this problem, but it would certainly cause even more annoying problems once this part is working. I personally blame Microsoft, it's much easier to tell that something declared as

Code: Select all

Foo *Bar;

is an object compared to

Code: Select all

LPFOO Bar

but unfortunately the compiler kicks up a huge fuss so I'm forced to use the second way even though it is just typedefed to be the same as the first.

Posted: **Wed Feb 01, 2006 6:26 am**

Kemp wrote: I think I've identified one other issue with my own copy of the object going out of scope at the end of the method, there are a couple of things in there on the ends of pointers that are freed when it is destroyed, and the pushed copy would still be holding the same pointers.

Hey, I asked you about deep vs. shallow copy. If you are holding pointers to something, it's the job of your copy constructor to make sure that either a refcount is taken or to make a deep copy. If you don't, your destructor will free memory that has already been free'd.

I don't think that is causing this problem, but it would certainly cause even more annoying problems once this part is working.

I certainly think that holding pointers to something without taking care of the fact in copy constructor and destructor is a capital offense in C++, punishable by crash.

I personally blame Microsoft, it's much easier to tell that something declared as
Code: Select all
Foo *Bar;
is an object compared to
Code: Select all
LPFOO Bar
but unfortunately the compiler kicks up a huge fuss so I'm forced to use the second way even though it is just typedefed to be the same as the first.

If it's typedef'ed to be the same as the first, then neither is an object. Both are pointers to an object.

Posted: **Wed Feb 01, 2006 9:25 am**

Ok, so I was typing that in a hurry before I left for a meeting, I meant pointer to an object, not just object

Hey, I asked you about deep vs. shallow copy. If you are holding pointers to something, it's the job of your copy constructor to make sure that either a refcount is taken or to make a deep copy. If you don't, your destructor will free memory that has already been free'd.

I know that, I was just saying I happened to miss the pointers when I was looking through before because of a combination of the layers of objects being contained in each other and that annoying scheme of Microsoft's to pretend you're not really declaring a pointer.

Posted: **Wed Feb 01, 2006 10:15 am**

Sorry for having sounded a bit rude, but playing "twenty questions" isn't fun when you get the impression the other person isn't giving the correct answers.

I still think you got your new / delete / copy constructors / destructors wrong somewhere, but as I said, without having the source it's only guesswork.

Posted: **Thu Feb 02, 2006 3:22 pm**

A question for those more knowledgable about these things (that'd be most of you probably...). Say you have:

Code: Select all

class Class1
{
// Blah blah blah
}

class Class2
{
private:
    Class1 *Foo;
}

class Class3
{
private:
    Class2 Bar;
}

And you wanted to push an instance of Class3 onto a vector (thus requiring a copy constructor). Then your original copy goes out of scope and is destroyed, thus also destroying its instance of Class2 (as it's part of the Class3) and manually destroying the Class1 that Class2 holds a pointer to. Does Class3's copy constructor have to handle making sure there is still a Class1 hanging around for the Class3/Class2 copy to use, or is Class2's copy constructor also called due to it being part of Class3?

Sorry about these questions, but my uni changed over to a useful language the year after I did the relevant modules :'( So basically my knowledge of these things is taught to me by google and a few books, and they don't like covering things beyond relatively simple cases.

Posted: **Thu Feb 02, 2006 11:57 pm**

Kemp wrote: Does Class3's copy constructor have to handle making sure there is still a Class1 hanging around for the Class3/Class2 copy to use, or is Class2's copy constructor also called due to it being part of Class3?

Class2's copy constructor is being called. It's the constructor of Class2 that should "populate" that pointer, its copy constructor that should do a deep copy, and its destructor that should do the delete.

For the purposes of Class3, Class2 is just another object that is created and goes out of scope. Whatever is in Class2 is up to Class2 (information hiding).

Posted: **Fri Feb 03, 2006 1:40 am**

Code: Select all

#include <iostream>
using namespace std;

class Class1
{
// Blah blah blah
  public:
  Class1() {}
  ~Class1() {}
  Class1(Class1 &rhs) {
    cout << "in copy constructor of class1" << endl;
    /*nothing */
  }
}

class Class2
{
  public:
  Class2() { Foo = new Class1(); }
  ~Class2() { delete Foo; }
  Class2(Class2 &rhs) {
    cout << "in copy constructor of class2" << endl;
    Foo = new Class1(rhs.Foo);
  }
private:
    Class1 *Foo;
}

class Class3
{
  public: 
  Class3() {}
  ~Class3() {}
  Class3(Class3 &rhs) {
    cout << "In copy constructor of class3" << endl;
    /* object auto-duplicates itself using copy constructor */
  }

private:
    Class2 Bar;
}

int main() {
  Class3 obj1;
  Class3 obj2 = obj1;
}

Try something like this. I'm not certain on the copy constructor, the argument might be const (probably is, but then again I'm not sure).

[edit] ok, here are the normal constructors and destructors too. [/edit]

Posted: **Fri Feb 03, 2006 2:28 am**

@ Candy:

Class is like struct - you have to terminate with a semicolon. And you need to dereference rhs.Foo in the Class2 copy constructor (rhs.Foo is a pointer, Class1 constructor requires an object).

And because you explicitly defined a copy constructor for Class3, you have to call the copy constructor of Class2 yourself. (This is automatic only if you do not declare a copy constructor.)

Code: Select all

  Class3(Class3 &rhs) : Bar( rhs.Bar ) {
    cout << "In copy constructor of class3" << endl;
  }

Hint for the confused beginner: if you want to make sure there isn't any copying goping on where you don't want it, declare the copy constructor as private and to not define it. That way, you will get a compiler error if it is called anywhere implicitly.

Posted: **Fri Feb 03, 2006 2:32 am**

Solar wrote: @ Candy:

Class is like struct - you have to terminate with a semicolon.

Look at Kemp's code, was a literal copy. Not my fault :-\

And you need to dereference rhs.Foo in the Class2 copy constructor (rhs.Foo is a pointer, Class1 constructor requires an object).

Well... ok, that'd be my kind of mistake.

And because you explicitly defined a copy constructor for Class3, you have to call the copy constructor of Class2 yourself. (This is automatic only if you do not declare a copy constructor.)

Never code in a hurry...

Posted: **Fri Feb 03, 2006 3:04 am**

Candy wrote: Look at Kemp's code, was a literal copy. Not my fault :-\

Kemp's was a snippet, yours was complete, so I fed it to the compiler and got error messages. I'm far too lazy to look for stuff like that myself.

Never code in a hurry...

My motto is: Posting code is like making a release. Never release untested code.

Posted: **Fri Feb 03, 2006 4:29 am**

And suddenly it dawns on me that there is one really easy and really obvious solution that never quite occurred to me... Create the object using 'new' and then push a pointer to it onto the vector. No copying involved and much, MUCH simpler to debug in the end. Another case if what seems natural (you want a vector of objects, so make objects and put them on the vector) not actually being what you want.

I'm gonna go try that method, if it works I'll celebrate a bit and then do a lot of reading on the topics covered here and try to nail them as much as I can in my head.

Posted: **Fri Feb 03, 2006 4:36 am**

If you have problems tracking down pointer-related problems in your class hierachy, circumventing the problem by using pointers elsewhere isn't really a "solution".

The beauty of a vector holding objects is that they get destroyed automatically together with the vector. If the vector holds only pointers, you have to take care of the deleting yourself, before the vector goes out of scope.

A rule that applies to many areas of software engineering: Don't work around a problem before you've really understood it.

Posted: **Fri Feb 03, 2006 6:04 am**

What you say is very true. My current problem is that I'm on a very "Make something that works, if only by accident, and then make it a good system" sort of schedule. Luckily I can mangle deadlines to my will to a limited extent, so on second thoughts I think I'll go with the proper way of doing this and make it actually work rather than working around the problem. I think I've figured out all I need to know to give my classes proper copy constructors though I'm still not sure that's the reason for my original problem, will get back to you (hopefully to tell you it works). Thanks for all the help guys.

Posted: **Fri Feb 03, 2006 8:35 am**

Candy wrote:

Code: Select all

  Class2(Class2 &rhs) {
    cout << "in copy constructor of class2" << endl;
    Foo = new Class1(rhs.Foo);
  }

There's a memory leak in that copy constructor.

@Kemp:
I agree with Solar that using a vector of bare new'd pointers is a pretty bad idea. However, I've been known to use a vector of boost::shared_ptr's instead if it makes sense (see www.boost.org).

IMO, you should only define a copy constructor and assignment operator for a class if you're really sure you want value semantics. For example, it makes little sense to be able to clone objects representing database connections or algorithms, but perhaps a lot of sense to clone objects representing complex numbers. Food for thought...

Posted: **Fri Feb 03, 2006 9:01 am**

Colonel Kernel wrote:

Code: Select all

  Class2(Class2 &rhs) {
    cout << "in copy constructor of class2" << endl;
    Foo = new Class1(rhs.Foo);
  }

There's a memory leak in that copy constructor.

Erm... rhs.Foo is not dereferenced (see my earlier post), but I don't see a memory leak?

Since the copy constructor of Class1 does nothing with the argument it is passed - specifically it is not creating a new reference to rhs.Foo - I can't see where it should be faulty? Nonsense perhaps, but no memory leak!?

OSDev.org

VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem

Re:VC++ STL Vector problem