Do i need a copy contructor?

[Simon]

Hi,

if I have a structure like...

struct sMyPointers{
ClassA *m_pPointerA;
ClassB *m_pPointerB;
void *m_pPointerToSomethingElse;
};

And then

std::vector<sMyPointers *> pPointers;

sMyPointers *pA = new sMyPointers();
pPointers->push_back(pA);
pA = new sMyPointers();
pPointers->push_back(pA);
pA = new sMyPointers();
pPointers->push_back(pA);

The reason why I want pointers is mainly or speed but also to make sure that
I only have one set of data, rather than a bunch of copies all over the
place.
The data is created one and deleted once.

But is the above correct? Is it good coding practice?

regards.
Simon

 

[Torsten Mueller]

struct sMyPointers{
ClassA *m_pPointerA;
ClassB *m_pPointerB;
void *m_pPointerToSomethingElse;
};

Where is the initialization of the pointers done? And especially:
where are the pointers deleted? Do you have a destructor doing this?
This question is relevant for answering on "Do I need a copy
constructor?" If these instances of sMyPointers own the inner
pointers, I would suggest to make a copy constructor.

std::vector<sMyPointers *> pPointers;

sMyPointers *pA = new sMyPointers();
pPointers->push_back(pA);
pA = new sMyPointers();
pPointers->push_back(pA);
pA = new sMyPointers();
pPointers->push_back(pA);

This is legal. You could avoid the variable pA. You do not need a copy
constructor here.

The reason why I want pointers is mainly or speed but also to make
sure that I only have one set of data, rather than a bunch of copies
all over the place. The data is created one and deleted once.

But is the above correct? Is it good coding practice?

Notice, the allocation and deallocation of objects on the heap can
take more time than to make a copy. It depends on the objects and
their data.

T.M.

 

[Bart]

struct sMyPointers{
ClassA *m_pPointerA;
ClassB *m_pPointerB;
void *m_pPointerToSomethingElse;
};

The real question is "Who owns the resources that the pointers points
to?"

Is this struct just a "holder" for grouping three pieces of data
together, or is it actually responsible for managing the resources? In
general, whichever class does the actual resource management should
have a copy constructor, destructor, and assignment operator.

Technically, this struct is okay for use in standard containers, but if
you want your code to be reliable you really have to answer the above
questions.

 

[Simon]

The real question is "Who owns the resources that the pointers points
to?"

Is this struct just a "holder" for grouping three pieces of data
together, or is it actually responsible for managing the resources? In
general, whichever class does the actual resource management should
have a copy constructor, destructor, and assignment operator.

Technically, this struct is okay for use in standard containers, but if
you want your code to be reliable you really have to answer the above
questions.

Well my class is in charge of the pointers, so it will create them and
delete them as needed.
The idea was to 'tidy' the code a little, rather than having

The number of items in the vector is unknown, so it is nice to be able to
create them on the fly and just add them to a structure.
That way I can manage the data better.

By having the structure, (and then the vector), the number of items is
immaterial.
I can create 5, 10, 100 instances as long as I delete every single items.

Simon.

 

[Jonathan Mcdougall]

Well my class is in charge of the pointers, so it will create them and

delete them as needed.
The idea was to 'tidy' the code a little, rather than having

The number of items in the vector is unknown, so it is nice to be able to
create them on the fly and just add them to a structure.
That way I can manage the data better.

By having the structure, (and then the vector), the number of items is
immaterial.
I can create 5, 10, 100 instances as long as I delete every single items.

Imagine this:

class C
{
private:
int *i;

public:
C()
: i(new int)
{
}

~C()
{
delete i;
}
};

int main()
{
C c1;
C c2(c1);

// here, both c1 and c2 have their pointer 'i' pointing to the same
// int
}

When c2 goes out of scope, it deletes its int. Then c1 goes out of
scope and the deletes the very same one, which is undefined behavior.

When a class manages memory, it usually needs a copy constructor and an
assignment operator:

class C
{
private:
int *i;

public:
C()
: i(new int)
{
}

C(const C &c)
: i(new int(c.i))
{
}

C &operator=(const C &c)
{
delete i;
i = new int(c.i);
}

~C()
{
delete i;
}
};

But you'd be better with a kind of smart pointer, which manages this
automatically. std::auto_ptr does not.


Jonathan

 

[Jonathan Mcdougall]

Just wanted to know if you were paying attention... The second code
should be

class C
{
private:
int *i;

public:
C()
: i(new int)
{
}

C(const C &c)
: i(new int(*c.i))
{
}

C &operator=(const C &c)
{
delete i;
i = new int(*c.i);

return *this;
}

~C()
{
delete i;
}
};


Jonathan

 

[Simon]

Just wanted to know if you were paying attention... The second code
should be

Thanks, I didn't want to say anything.

but while we are on the subject, isn't

const C &operator=(const C &c)
....

better than

C &operator=(const C &c)


?

Simon

 

[Clark S. Cox III]

Thanks, I didn't want to say anything.

but while we are on the subject, isn't

const C &operator=(const C &c)
...

better than

C &operator=(const C &c)

No, why do you think it would be? Generally, when overloading
operators, a good goal is to make them behave as closely as possible to
how they would behave when applied to built-in types; using the
assignment operator on a built in type results in what is essentially a
non-const reference to the left operand:

int i;
int &ir = i = 5; //Assigns 5 to i, and binds ir to i

 

[Simon]

No, why do you think it would be? Generally, when overloading operators, a
good goal is to make them behave as closely as possible to how they would
behave when applied to built-in types; using the assignment operator on a
built in type results in what is essentially a non-const reference to the
left operand:

int i;
int &ir = i = 5; //Assigns 5 to i, and binds ir to i

Truth be told, I am not sure, I remember reading somewhere that it was
better to do the "const", but I don't know where I got such an idea from.

Maybe someone could help me on that one.

Simon

 

[Jonathan Mcdougall]

Truth be told, I am not sure, I remember reading somewhere that it was

better to do the "const", but I don't know where I got such an idea from.

Maybe someone could help me on that one.

Well for myself, I remember reading that void main() was better, so you
can't count on that.

An argument you may have heard is that it allows

(c1=c2) = c3;

which looks weird, but is allowed for ints:

int i1=1, i2=2, i3=3;

(i1=i2)=i3;

As our good Scott Meyers said, "do as the ints do".

Jonathan