# copy constructor and assignment operator

## Recommended Posts

Hello ppl, i 'd like to ask about the correctness of the assignment operator and copy constructor defined in the following piece of code. I have written a small test for self-assignment and it think it's correct but i want to be sure. Is there any case i 'm missing?

namespace Map2D
{
namespace Color
{

class UIntRGBA8
{
public:
struct Element
{
UInt8 R;
UInt8 G;
UInt8 B;
UInt8 A;

Element(){};

Element(UInt8 _R, UInt8 _G, UInt8 _B, UInt8 _A)
{
R = _R;
G = _G;
B = _B;
A = _A;
};
};

private:
UInt32 width;
UInt32 height;

Element* pixels;

public:
UIntRGBA8(UInt32 _width, UInt32 _height)
{
width = _width;
height = _height;
pixels = new Element[width * height];
};

UIntRGBA8(const UIntRGBA8 &_UIntRGBA8)
{
width = _UIntRGBA8.width;
height = _UIntRGBA8.height;
pixels = new Element[width * height];
memcpy(pixels, _UIntRGBA8.pixels, sizeof(Element) * width * height);
};

UIntRGBA8& operator=(const UIntRGBA8 &_UIntRGBA8)
{
width = _UIntRGBA8.width;
height = _UIntRGBA8.height;
Element *tmp = new Element[_UIntRGBA8.width * _UIntRGBA8.height];
memcpy(tmp, _UIntRGBA8.pixels, sizeof(Element) * width * height);
delete[] pixels;
pixels = tmp;
return *this;
};

~UIntRGBA8()
{
delete[] pixels;
};
};

};

};



##### Share on other sites
Its correct in that it will work "normally" but its not exception safe. In operator=() if the allocation fails (i.e. new throws) then your object will be left in an inconsistent state.

Some points:
1) You don't have to do all that if you use std::vector<>
2) You shouldn't be using memcpy in c++, you should use std::copy instead
3) In general you should avoid names with a leading underscore as the c++ standard reserves them for the implementation in the global and std namespaces.

##### Share on other sites

I used to use the 'Src' suffix for my constructors' arguments but for some reason i changed it to underscore... I guess i 'll change it back :P

I 'll consider using the vector class, though i 'd first like to get the existing code to be correct.

If i change this:
UIntRGBA8& operator=(const UIntRGBA8 &_UIntRGBA8){	width = _UIntRGBA8.width;	height = _UIntRGBA8.height;	Element *tmp = new Element[_UIntRGBA8.width * _UIntRGBA8.height];	memcpy(tmp, _UIntRGBA8.pixels, sizeof(Element) * width * height);	delete[] pixels;	pixels = tmp; 	return *this;};

to this:
UIntRGBA8& operator=(const UIntRGBA8 &_UIntRGBA8){        Element *tmp = new Element[_UIntRGBA8.width * _UIntRGBA8.height];	width = _UIntRGBA8.width;	height = _UIntRGBA8.height;	memcpy(tmp, _UIntRGBA8.pixels, sizeof(Element) * width * height);	delete[] pixels;	pixels = tmp;	return *this;};

So that new throws before i do any changes to the object, am i safe? Or should i use some kind of exception handling?

##### Share on other sites
you should always check for the special case when you say
something like

UIntRGBA8 x;
x = x;

UIntRGBA8& operator=(const UIntRGBA8 &_UIntRGBA8)
{
if(this == &_UIntRGBA8) return *this;
// rest of code
}

one thing i've always thought funny was that in VS,
this is valid code even though it don't make any sense ha ha ha:

struct Object { Object(const Object&) {} };
Object obj(obj); // make a copy of an object that hasn't been constructed yet?

##### Share on other sites
Quote:
 I used to use the 'Src' suffix for my constructors' arguments but for some reason i changed it to underscore... I guess i 'll change it back :P

I'd consider why you actually need to put a suffix at all, for example the following is unambiguous:

struct foo{    // I forgot to mention in the first post, you should use    // initializer lists wherever possible    foo(int value) : value(value) { }private:    int value;};

And if you need to write a body for the constructor then you can use this->value to access member variables.

Quote:
 If i change this:So that new throws before i do any changes to the object, am i safe? Or should i use some kind of exception handling?

Yep, that should be safe.

Quote:
 you should always check for the special case when you saysomething likeUIntRGBA8 x;x = x;

The allocation of a new block of memory means that as his operator= is currently written it will work correctly in this case without special handling (which is what I interpreted his original question as asking)

Quote:
 struct Object { Object(const Object&) {} };Object obj(obj); // make a copy of an object that hasn't been constructed yet?

IIRC this results in undefined behavior regardless of weather you have written the copy constructor to deal with it.

## Create an account

Register a new account

• ## Partner Spotlight

• ### Forum Statistics

• Total Topics
627653
• Total Posts
2978440

• 10
• 12
• 22
• 13
• 33