Sign in to follow this  

Polynomial overloaded operator =

This topic is 3193 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

This is another post in reference to a polynomial using a queue style format and the problem I am having is how to program the overloaded operator = for the class. I think I am on the right track but it crashes everytime I try and run this program so I am kinda stuck.
void Polynomial::operator =(const Polynomial& p)
{
	if(p.head == NULL)
		head = tail = NULL;
	else
	{
		PolyPtr temp = p.head;
		head->coefficent = p.head->coefficent;
		head->power = p.head->power;
		head->link = NULL;
		tail = head;

		temp = temp->link;
		while(temp != NULL)
		{
			tail->link = temp;
			tail = temp;
		}
	}	
}

Here is what head,tail, and PolyPtr refer to
struct PolyNode
{
	int power;
	int coefficent;
	PolyNode *link;
};

typedef PolyNode *PolyPtr;

class Polynomial
{
public:
	// Constructors
	Polynomial();
	Polynomial(const Polynomial& p);
	Polynomial(int a);
	Polynomial(int a, int b);

	// Destructor
	~Polynomial();

	// overloaded = operator
	void  operator =(const Polynomial& p);

	// addition
	friend Polynomial operator +(const Polynomial& p, const Polynomial& p2);
	// subtraction
	friend Polynomial operator -(const Polynomial& p, const Polynomial& p2);
	// multiplication
	friend Polynomial operator *(const Polynomial& p, const Polynomial& p2);
	// input
	friend istream& operator >>(istream& ins, Polynomial& p);
	// outputss
	friend ostream& operator <<(ostream& outs, const Polynomial& p);

	// this will react just like a insert in a queue
	void insert(int c, int p);

	// evaluation
	double eval(int a);
	
private:
	PolyPtr head;
	PolyPtr tail;
};

Any help would be truly appreciated. I had been posting about operator + but I found out that I had totally missed the operator = operations so therefore until I have that I can not adequately test my operator + function. Thanks for any help.

Share this post


Link to post
Share on other sites
Found a couple things that muck up the code but still get the crash
This is the code that you should look at for the operator = function.


void Polynomial::operator =(const Polynomial& p)
{
if(p.head == NULL)
head = tail = NULL;
else
{
PolyPtr temp = p.head;
head->coefficent = temp->coefficent;
head->power = temp->power;
head->link = NULL;
tail = head;

temp = temp->link;
while(temp != NULL)
{
tail->link = temp;
tail = temp;
temp = temp->link;
}
}
}

Share this post


Link to post
Share on other sites
Firstly, I see no reason why your arithmetic operators couldn't be members instead of friends.

Secondly, I can't tell without seeing the constructor(s), but you might want to check if it's possible for 'head' to point to invalid memory. You're assuming that 'head' is valid in body polynomials, and copying the head without checking for null. Here's how I'd do this code (assume we're in the 'else'):

EDIT: I am an idiot. Your if/else checks for the head null thing. Well, at least this solves the deep/shallow copy problem and calls delete properly... the former of which could have caused the crash anyhow.


// delete the old polynomial
while (head != NULL)
{
PolyPtr temp = head->link;
delete head;
head = temp;
}

// Create at least one node to start
// If 'p' happens to be empty, we'll still
// have something valid in this one.
tail = new PolyNode;
tail->power = 1;
tail->coefficient = 0;

head = tail; // anchor the head to the new head
// we're using tail to add on
PolyPtr temp = p.head;

while (temp != NULL)
{
// copy the node temp is pointing to
// onto the node curr is pointing to
*tail = *temp;

// if there's another link still,
if (temp->link != NULL)
{
// create a new polynode
// we want to "deep-copy" the polynomial, so we have
// separate memory locations for tail and temp
tail->link = new PolyNode;

// move forward
tail = tail->link;
}
}





I haven't tested it, but it looks solid. Could probably be done better, too, but at least it should work. (I hope it does. O_o) You also might want to look into step-by-step debugging to see exactly where it crashes.

~Jonathan


EDIT: See edit above code.

[Edited by - Twisol on March 13, 2009 3:57:04 PM]

Share this post


Link to post
Share on other sites
Yeah, and although you did it differnetly Twisol you really helped me get back on the right track. Here is what I did but it's pretty similar to your suggestion.


void Polynomial::operator =(const Polynomial& p)
{
if(p.head == NULL)
head = tail = NULL;
else
{
PolyPtr temp = p.head;
head = new PolyNode;
head->coefficent = temp->coefficent;
head->power = temp->power;
head->link = NULL;
tail = head;

temp = temp->link;
while(temp != NULL)
{
PolyPtr temp2 = new PolyNode;
temp2->coefficent = temp->coefficent;
temp2->power = temp->power;
temp2->link = NULL;
tail->link = temp2;
tail = temp2;
temp = temp->link;
}
}
}

Share this post


Link to post
Share on other sites
Sure, that works. You're leaking memory though: every time you assign one polynomial to another, the previous contents of the polynomial on the left are lost and inaccessible, yet still allocated and containing information. That's why I made sure to run through the polynomial and delete everything first, before I copied the right polynomial over.

Memory leaks are a bad idea, and if your memory space gets too cluttered it slows down your program, even if most of that allocated memory is actually unused.

Glad I helped you out, though. [smile]
~Jonathan

Share this post


Link to post
Share on other sites
Try using the 'one true copy constructor': the copy-swap idiom.

Writing a swap function for two polynomials is easy.

Writing the copy constructor is easier than writing operator= (because the destructor already deals with destroying the elements).

Your operator= then looks like:

anyType& operator=(const anyType& other) {
anyType tmp = other;
this->swap(tmp);
return *this;
}



The above code will be as exception safe as your swap (and swap shouldn't throw), copy constructor and destructor. It works for almost every class that uses pointer or value semantics on swap (reference semantics on swap breaks it).

You have to write copy construct and destructors anyhow. And swap is easy to write. So you have just reduced code duplication.

And in most cases, there is next to no overhead from the temporary object created by the copy constructor.

And yes, you really should be thinking about using a std::list or the like. Writing a list by hand, as more than a learning exercise, is a bad idea.

...

For the copy constructor, I'd write:

PolyPtr clonePolyPtr( PolyPtr src ) {
if (!src) return src;
PolyPtr tmp = new PolyPtr();
tmp->coefficient = src->coefficient;
tmp->power = src->power;
tmp->link = clonePolyPtr( src->link );
return tmp;
}


(Note that making the above exception-safe is annoying. Which is another reason to use std::list.)

Now your copy constructor is '0' lines long:

Polynomial::Polynomial( const Polynomial& other ):head( clonePolyPtr(other.head) ) {}


Swap is easy:

void Polynomial::swap( Polynomial& other ) {
std::swap( head, other.head );
}


And .. done. Well you need to write Polynomial::~Polynomial, but you would have had to do that anyhow.

Going deeper down the rabbit hole, you could use boost::shared_ptr and make the cleanup code trivial (ie, a trivial destructor works).

Share this post


Link to post
Share on other sites

This topic is 3193 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