Sign in to follow this  

c++ assignment operator with pointers

This topic is 4867 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi again all, Okay, this worked before, but now it doesn't seem to like me very much. When I try to to assign from one array to another using this operator, I get an unhandled exception "Access violation writing location 0x00000004." that's being caused by the line "*this->parent = *original.ReturnParent();"
Tetris_Box & Tetris_Box::operator= (const Tetris_Box &original)
{
	if (this == &original)
	{
		return *this;
	}

	if (original.ReturnParent() == NULL)
	{
		this->parent = NULL;
	}
	else
	{
		*this->parent = *original.ReturnParent();
		this->parent->AddChild(this);
	}

	this->h = original.GetH();
	this->w = original.GetW();
	this->t = original.GetT();
	this->l = original.GetL();
	this->z = original.GetZ();
	this->background_image = original.GetBG();

	return *this;
}
If anyone has any suggestions as to why it's not liking me copying that pointer over? Thanks much!

Share this post


Link to post
Share on other sites
*this->parent = *original.ReturnParent();

You have checked the validity of original.ReturnParent(), but not that of this->parent. Yet, you are trying to write to the object that this->parent points to.

You are not assigning pointers, you are assigning objects.

Share this post


Link to post
Share on other sites
I'm rather new to using pointers and I don't totally understand them yet. I was told in here that this method is what I wanted... Thanks for helping me Fruny, and piss off, AP ;)

Share this post


Link to post
Share on other sites
Quote:
Original post by Instruo
I'm rather new to using pointers and I don't totally understand them yet. I was told in here that this method is what I wanted... Thanks for helping me Fruny, and piss off, AP ;)


Yeah, but look at your code. Imagine that first you assign a node which has no parent. So you do this->parent = NULL;. Then later, you assign a node which has a parent, so you do *this->parent = *original.ReturnParent();. But this->parent is a null pointer, so your program explodes.

That is the problem, as I see it. If the node you are assigning to doesn't have a parent, you have to do *something*. :)

Share this post


Link to post
Share on other sites
I don't know how efficient this is (i would think it's quite bad as you need to create a temporary object), but it's very easy to implement and deals with self assignements.
Here's my general assignment operator (you must make sure that you copy constructor works first though!!)


#include <algorithm>

ClassType & operator=(const ClassType & rhs)
{
ClassType temp(rhs); //Create a copy
std::swap(someVariable, temp.someVariable); //Swap variables
// std::swap rest of member variables
return *this;
} //temp will get automatically cleaned up




I found it somewhere on the internet, can't remeber where now. Should work OK in most situations. Let me know if there are any problems.

Share this post


Link to post
Share on other sites
It appears that he intends that returnParent() will return a pointer which he is dereferencing (to make a copy of the Parent object within the current block). So no.

Could this possibly do what you're looking for, OP?

delete this->parent; /* get rid of existing parent; does nothing if this->parent was NULL */
this->parent = new Parent(original.ReturnParent()); /* invoke copy constructor */


Alternately, design ReturnParent() so that it makes a copy and returns a pointer to the copy. Although in this case you will want to make sure you only call it once; i.e. cache the result of the if-condition and use that directly in the else-case.

Share this post


Link to post
Share on other sites
Replace this line:
Quote:


*this->parent = *original.ReturnParent();

with this:

this->parent = original.ReturnParent();

That will make this->parent point to the same object as original's parent. I'm sure that's what you want.

Share this post


Link to post
Share on other sites

This topic is 4867 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this