Back to For Beginners

Polynomial overloaded operator =

For Beginners

Started by toogreat4u March 13, 2009 02:29 PM

5 comments, last by NotAYakk 15 years, 1 month ago

toogreat4u

127

Author

March 13, 2009 02:29 PM

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.

toogreat4u

127

Author

March 13, 2009 02:34 PM

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;		}	}	}

Blobberson

116

March 13, 2009 02:44 PM

Can I ask why you're doing the list management yourself and not using a std::list<PolyNode> inside your polynomial class?

Twisol

468

March 13, 2009 02:57 PM

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 polynomialwhile (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 onPolyPtr 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]

toogreat4u

127

Author

March 13, 2009 04:27 PM

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;		}	}	}

Twisol

468

March 13, 2009 05:33 PM

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

NotAYakk

876

March 16, 2009 08:46 AM

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).

Polynomial overloaded operator =

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Polynomial overloaded operator =

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines