-2

Hello I am working on a polyminal class. I have a problem with the copy (=) operator overloading.Let me give some examples.

Let's say i have:

P = 2x^6 + 4x^5 + 2x^3 + 3x^2 + 2x + 1

if i do P=P I get:

P 4x^6 + 8x^5 + 4x^3 + 6x^2 + 4x + 2

I also get the thing with P = Q*P. I don't think my method has any problems, maybe something else.

#include <iostream>
#include <cmath>

using namespace std;

class Polynomial {
protected:
    class Term {
    public:
        int exponent;
        int coefficient;
        Term *next;

        Term(int exp, int coeff, Term *n) {
            exponent = exp;
            coefficient = coeff;
            next = n;
        }
        ~Term(){
            delete next;
        }

        friend class Polynomial;

        friend ostream;
    };

public:
    Polynomial() {
        root = NULL;
    }

    Polynomial(const Polynomial &p) {
        Term *target = p.root;
        root = NULL;
        while (target != NULL) {
            this->addTerm(target->exponent, target->coefficient);
            target = target->next;
        }
    }

    ~Polynomial() {
        delete root;
    }

    Polynomial & operator = (const Polynomial &p){
        Term *target = p.root;
        while (target!=NULL){
            this->addTerm(target->exponent , target->coefficient);
            target = target->next;
        }
        return *this;
    }

    void addTerm(int expon, int coeff) {

        Term *prev = root;
        Term *target = root;

        if (root == NULL) {
            Term *p = new Term(expon, coeff, target);
            //Term *p = &tmp;
            root = p;
            return;
        }

        while (target != NULL && target->exponent > expon) {
            prev = target;
            target = target->next;
        }

        if (target && target->exponent == expon) {
            target->coefficient += coeff;
            if (target->coefficient == 0) {
                prev->next = target->next;
            }
            return;
        }

        if (target == root) {
            Term *p = new Term(expon, coeff, target);
            root = p;   //Set p as root
        } else {

            Term *p = new Term(expon, coeff, target);
            prev->next = p;


        }


    }

    double evaluate(double x) {
        Term *target = root;
        double sum=0;
        while (target != NULL) {
            sum += target->coefficient * pow(x, target->exponent);
            target = target->next;
        }
        return sum;
    }

    friend Polynomial operator+(const Polynomial &p, const Polynomial &q) {
        Polynomial tmp;
        Term *target = p.root;

        while (target != NULL) {
            tmp.addTerm(target->exponent, target->coefficient);
            target = target->next;

        }
        target = q.root;
        while (target != NULL) {
            tmp.addTerm(target->exponent, target->coefficient);
            target = target->next;
        }
        return tmp;

    }

    friend Polynomial operator * (const Polynomial &p, const Polynomial &q) {
        Polynomial tmp;
        Term *target_1 = p.root;
        Term *target_2 = q.root;
        while (target_1 != NULL) {
            while (target_2 != NULL) {

                tmp.addTerm(target_1->exponent + target_2->exponent, target_1->coefficient * target_2->coefficient);

                target_2 = target_2->next;
            }
            target_2 = q.root;
            target_1 = target_1->next;
        }
        return tmp;
    }


    friend ostream &operator << (ostream &out, const Polynomial &p) {
        Term *target = p.root;
        if (target!=NULL && target->coefficient < 0) cout<<"- ";

        while (target != NULL) {
            if (target->exponent){
                if (abs(target->coefficient) != 1) out << abs(target->coefficient);
            }
            else {
                out << abs(target->coefficient);
            }
            out << (target->exponent ? (target->exponent != 1 ? "x^" : "x") : "");
            if (target->exponent) {
                if (target->exponent != 1) out << target->exponent;
            }
            target = target->next;
            if (target!= NULL) {
                if (target->coefficient > 0){
                    out<<" + ";
                }
                else{
                    out <<" - ";
                }
            }


        }


        return out;
    }

private:
    Term *root;
};

#ifndef CONTEST
int main(){

    Polynomial P;
    P.addTerm(6,2);
    P.addTerm(5,4);
    P.addTerm(3,2);
    P.addTerm(2,2);
    P.addTerm(1,2);
    P.addTerm(2,1);
    P.addTerm(0,1);

    cout<<P<<endl;

    cout<<P.evaluate(0)<<endl;
   // Polynomial Q = P;

    //Polynomial S = Q*P;
    P = P;

    //cout<<S<<endl;
    cout<<P<<endl;
}

#endif
  • Maybe it's just me, but in your copyconstructor and in your `=` operator definition, you do `Term *target = p.root;` and then do `while (target!=NULL){ this->addTerm(target->exponent , target->coefficient);`. Doesn;t that add the RHS to the LHS if the LHS is not an empty polynomial? – Srini Apr 23 '18 at 17:25

1 Answers1

1

Your operator= is incorrect: assignment should replace the terms in the current polynomial, not add to them. You need to first clear the object being assigned to:

delete root;
root = nullptr;

Doing this requires protecting against self-assignment. So start your assignment operator with this:

if (this == &p) return *this;

Then clear *this, and then copy p as you do.


An alternative solution would be to reuse the work you already did for the copy constructor and use the copy-and-swap idiom for the assignment operator. This requires:

  1. Defining a swap function for your class:

    friend void swap(Polynomial &lhs, Polynomial &rhs)
    {
      std::swap(lhs.root, rhs.root);
    }
    
  2. Changing the operator= to take its argument by value, and swap with it inside:

    Polynomial& operator=(Polynomial p)
    {
      swap(*this, p);
      return *this;
    }
    

This way, the object p will initialised created by the copy constructor, then just swapped with the polynomial being assigned to (transferring its value along the way), and finally destroyed when operator= ends, conveniently taking care of cleaning up the old value of the assigned-to polynomial.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • I get what I said. Could i just create a new Polyminal in the operator= ?. Which is more optimal – Stavros Avramidis Apr 23 '18 at 17:28
  • @StavrosAvramidis One common way of easily implementing `operator=` is the [copy-and-swap idiom](https://stackoverflow.com/q/3279543/1782465). You'd just need to implement `swap` for your class (dead simple swap of the two roots), take the `operator=` argument by value, and swap with it. – Angew is no longer proud of SO Apr 23 '18 at 17:30
  • Ok, thank you!!, By the my destructor is ok right?? I had one guy complaining about it but deleted his answer..? – Stavros Avramidis Apr 23 '18 at 17:38