Why would my overloaded operator+ make 'this' uninitialized on return when adding a variable to itself?

Question

I've found a bug in my code in which the line in my driver

t2 = t2 + t;

results in 'this' becoming uninitialized during the return of my overloaded operator+. Using the debug tool my code seems to execute perfectly fine up until the return call, then when I step inside return it takes me to my copy constructor with 'this' pointing to the same head node as the p parameter passed into it. But once I step over the copy constructor header into the body, 'this' suddenly changes it's memory location and points to 0xcccccccc, telling me it's not initialized, which causes issues in my constructor since that isn't a null value.

I was hoping someone could give me some insight into what exactly is going on, and how to fix it. Obviously I could just use my operator+= since it works just fine, but in theory you should be able to make this kind of call without errors.

EDIT: After getting comments that pointed out glaring mistakes that I did not know to do better, I have edited my code to include helper copy and clear methods to use in my copy constructor, destructor, and assignment operator.

Updated Implementation:

#include "polynomial.h" //header
#include <stdlib.h>

// Default Constructor:the default is a 0-degree polynomial with 0.0 coefficient

Polynomial::Polynomial() {
    size = 0;
    head = new Term;
    head->coeff = 0;
    head->power = 0;
    head->next = head;
    head->prev = head;
}

// Copy Constructor

Polynomial::Polynomial(const Polynomial &p) {
    copy(p);
}

// Destructor

Polynomial::~Polynomial() {
    clear();
}

// degree: returns degree of polynomial
// Pre: none (an empty polynomial will return as a 0-degree)
// Post: the largest degree in the polynomial is returned as an int

int Polynomial::degree() const {
    return head->next->power;
}

// coefficient: returns the coefficient of the x^power Term
// Pre: none
// Post: the coefficient, if an x^power Term exists, is return. If 
//      an x^power Term doesn't exist, 0 is returned

double Polynomial::coefficient(const int power) const {
    Term *thisPointer = head->next;
    while (thisPointer->power != power && thisPointer != head) thisPointer = thisPointer->next;
    if (thisPointer == head) return 0;
    return thisPointer->coeff;
}

// changeCoefficient: replaces the coefficient of the x^power term
// Pre: none
// Post: if an x^power Term exists, its coefficient will be changed to 
//      newCoefficient. If not, an x^power Term with that coefficient will be inserted

bool Polynomial::changeCoefficient(const double newCoefficient, const int power) {
    if (head == NULL) *this = Polynomial();
    Term *thisPointer = head->next;
    // either finds value to be changed or stops before head, indicating need for insertion
    while (thisPointer->power != power && thisPointer->next != head)
        thisPointer = thisPointer->next;
    // finds proper location for insertion if cycled through list
    if (thisPointer->next == head) {
        thisPointer = head;
        while (thisPointer->next->power > power) thisPointer = thisPointer->next;
        insert(thisPointer->next, newCoefficient, power);
    }
    else if (newCoefficient == 0) {
        remove(thisPointer);
    } else thisPointer->coeff = newCoefficient;
    return true;
}

// insert: inserts an x^power Term with coefficient newCoefficient into the
//      polynomial, directly just before the pos Term
// Pre: the function is passed a nonzero newCoefficient and a pos Term that
//      exists in the polynomial
// Post: returns true if new Term was successfully inserted, returns false if
//      pre conditions are not met

bool Polynomial::insert(Term* pos, const double newCoefficient, const int power) {
    if (newCoefficient == 0) return false;
    Term *thisPointer = head;
    while (thisPointer->next != pos && thisPointer->next != head)
        thisPointer = thisPointer->next;
    // returns false if pos Term is not found
    if (size > 0 && thisPointer->next == head && pos != head) return false;
    // creates new term using parameters
    Term *newTerm = new Term;
    newTerm->power = power;
    newTerm->coeff = newCoefficient;
    // redirects pointers of adjacent Terms to include newTerm in polynomial
    newTerm->next = thisPointer->next;
    newTerm->prev = thisPointer;
    thisPointer->next->prev = newTerm;
    thisPointer->next = newTerm;
    size++;
    return true;
}

// remove: removes pos Term from polynomial
// Pre: pos Term exists in polynomial
// Post: returns true if pos Term was successfuly removed, if not returns false

bool Polynomial::remove(Term* pos) {
    Term *thisPointer = head;
    // finds term before one to be deleted
    while (thisPointer->next != pos && thisPointer->next != head) thisPointer = thisPointer->next;
    //returns false if pos term is not found
    if (thisPointer->next == head) return false;
    // redirects pointers of adjacent Terms around pos, removes pos
    thisPointer->next = thisPointer->next->next;
    thisPointer->next->prev->prev = NULL;
    thisPointer->next->prev->next = NULL;
    delete thisPointer->next->prev;
    thisPointer->next->prev = thisPointer;
    size--;
    return true;
}

// clear: removes all terms of polynomial from memory
// Pre:
// Post: 
void Polynomial::clear() {
    if (head != NULL) {
        while (head->next != head) {
            Term *thisPointer = head;
            head = head->next;
            remove(thisPointer);
        }
        head->next = head;
        head->prev = head;
        head->coeff = 0;
    }
}

// copy:
// Pre:
// Post: 
void Polynomial::copy(const Polynomial &p) {
    Term *pPointer = p.head;
    if (pPointer == NULL) head = NULL;
    else {
        head = new Term;
        head->coeff = 0;
        head->power = 0;
        head->next = head;
        head->prev = head;
        size = p.size;
        //copy rest of polynomial
        Term *thisPointer = head;
        while (pPointer->next != p.head) {
            pPointer = pPointer->next;
            double coeff = pPointer->coeff;
            int power = pPointer->power;
            // create new Term with data from Term in p
            Term *newTerm = new Term;
            thisPointer->next = newTerm;
            newTerm->prev = thisPointer;
            newTerm->coeff = coeff;
            newTerm->power = power;
            thisPointer = thisPointer->next;
            // checks for end of p to link head and last term
            if (pPointer->next == p.head) {
                head->prev = thisPointer;
                thisPointer->next = head;
            }
        }
    }
}

// Overloaded <<: prints Cn * x^n + Cn-1 * X^n-1 + ... C1 * X + C0

ostream& operator<<(ostream &output, const Polynomial& p) {
    Polynomial::Term *thisPointer = p.head->next;
    if (thisPointer != NULL) {
        while (thisPointer != p.head) {
            if (thisPointer->coeff < 0) output << "-";
            else if (thisPointer->prev != p.head) output << " + ";
            if (abs(thisPointer->coeff) != 1) output << abs(thisPointer->coeff);
            if (abs(thisPointer->power) == 1) output << "x";
            else if(thisPointer->power != 0) output << "x^" << thisPointer->power;
            thisPointer = thisPointer->next;
        }
    }
    return output;
}

// Overloaded operator+

Polynomial Polynomial::operator+(const Polynomial &in) const {
    Polynomial out = *this;
    out += in;
    return out;
}

// Overloaded operator-

Polynomial Polynomial::operator-(const Polynomial &in) const {
    Polynomial out = *this;
    out -= in;
    return out;
}

// Overloaded operator==

bool Polynomial::operator==(const Polynomial &in) const {
    Term *thisPointer = head->next;
    Term *inPointer = in.head->next;
    while (thisPointer != head || inPointer != in.head) {
        if (thisPointer->power != inPointer->power || 
            thisPointer->coeff != inPointer->coeff) return false;
        thisPointer = thisPointer->next;
        inPointer = inPointer->next;
    }
    return true;
}

// Overloaded operator!=

bool Polynomial::operator!=(const Polynomial &in) const {
    return !(*this == in);
}

// Overloaded operator=

Polynomial& Polynomial::operator=(const Polynomial &p) {
    if (*this != p) {
        clear();
        copy(p);
    }
    return *this;
}

// Overloaded operator+=

Polynomial& Polynomial::operator+=(const Polynomial &in) {
    Term *thisPointer = head;
    Term *inPointer = in.head;
    while (thisPointer->next != head || inPointer->next != in.head) {
        int power = inPointer->next->power;
        double coeff = inPointer->next->coeff;
        // if t > p, insert t in p
        if (power > thisPointer->next->power) {
            insert(thisPointer->next, coeff, power);
            thisPointer = thisPointer->next;
        }
        // if p power = t power, add 
        else if (power == thisPointer->next->power)
            thisPointer->next->coeff += coeff;
        if (thisPointer->next->coeff == 0) remove(thisPointer->next);
        // only advances t if p->next isn't larger than t->next
        if (inPointer->next->power <= thisPointer->next->power)
            thisPointer = thisPointer->next;
        inPointer = inPointer->next;
    }
    return *this;
}

// Overloaded operator-=

Polynomial& Polynomial::operator-=(const Polynomial &in) {
    Term *thisPointer = head;
    Term *inPointer = in.head;
    while (thisPointer->next != head || inPointer->next != in.head) {
        int power = inPointer->next->power;
        double coeff = inPointer->next->coeff;
        // if t > p, insert t in p
        if (power > thisPointer->next->power) {
            insert(thisPointer->next, -coeff, power);
            thisPointer = thisPointer->next;
        }
        // if p power = t power, subtract 
        else if (power == thisPointer->next->power)
            thisPointer->next->coeff -= coeff;
        if (thisPointer->next->coeff == 0) remove(thisPointer->next);
        // only advances t if p->next isn't larger than t->next
        if (inPointer->next->power <= thisPointer->next->power)
            thisPointer = thisPointer->next;
        inPointer = inPointer->next;
    }
    return *this;
}

Driver file:

#include <iostream>
using namespace std;

#include "polynomial.h"

int main() {
    Polynomial t;

    // adds new Terms using changeCoefficient
    t.changeCoefficient(1, 1);
    t.changeCoefficient(2, 2);
    t.changeCoefficient(3, 3);
    t.changeCoefficient(4, 5);
    t.changeCoefficient(-5, 4);

    cout << "t = " << t << endl;
    // degree() demonstration
    cout << "t's degree = " << t.degree() << endl;

    // changing coefficients/deleting terms/adding constant value
    t.changeCoefficient(1, 3);
    t.changeCoefficient(9, 0);
    t.changeCoefficient(0, 5);

    cout << "t = " << t << endl;
    cout << "t's degree = " << t.degree() << endl;

    // copy constructor and operator= demonstration
    Polynomial t2 = t + t;
    // operator+ demonstration
    t2 = t2 + t;

    cout << "t2 = " << t2 << endl;

    // operator!= demonstration
    cout << "t2 does not equal t, right? " << boolalpha << (t2 != t) << endl;

    // operator- demonstration
    Polynomial t3 = t - t2;

    cout << "t3 = " << t3 << endl;

    // operator-= demonstration
    t2 -= t;

    cout << "t2 = " << t2 << endl;
    // operator== demonstration
    cout << "Does t2 equal t now? " << boolalpha << (t2 == t) << endl;

    // operator+= demonstration, tests that subtraction was done properly, and
    // t3 = empty polynomial
    t3 += t;

    cout << "t3 = " << t3 << endl;

}

OLD Implementation file:

#include "polynomial.h" //header
#include <stdlib.h>

// Default Constructor:the default is a 0-degree polynomial with 0.0 coefficient

Polynomial::Polynomial() {
    size = 0;
    head = new Term;
    head->coeff = 0;
    head->power = 0;
    head->next = head;
    head->prev = head;
}

// Copy Constructor

Polynomial::Polynomial(const Polynomial &p) {
    Term *pPointer = p.head;
    if (pPointer == NULL) head = NULL;
    else if (this != &p) {
        if (head != NULL) this->~Polynomial();
        head = new Term;
        head->coeff = 0;
        head->power = 0;
        head->next = head;
        head->prev = head;
        size = p.size;
        //copy rest of polynomial
        Term *thisPointer = head;
        while (pPointer->next != p.head) {
            pPointer = pPointer->next;
            double coeff = pPointer->coeff;
            int power = pPointer->power;
            // create new Term with data from Term in p
            Term *newTerm = new Term;
            thisPointer->next = newTerm;
            newTerm->prev = thisPointer;
            newTerm->coeff = coeff;
            newTerm->power = power;
            thisPointer = thisPointer->next;
            // checks for end of p to link head and last term
            if (pPointer->next == p.head) {
                head->prev = thisPointer;
                thisPointer->next = head;
            }
        }
    }
}

// Destructor

Polynomial::~Polynomial() {
    if (head != NULL) {
        while (head->next != head) {
            Term *thisPointer = head;
            head = head->next;
            remove(thisPointer);
        }
        head->next = head;
        head->prev = head;
        head->coeff = 0;
        size = 0;
    }
}

// degree: returns degree of polynomial
// Pre: none (an empty polynomial will return as a 0-degree)
// Post: the largest degree in the polynomial is returned as an int

int Polynomial::degree() const {
    return head->next->power;
}

// coefficient: returns the coefficient of the x^power Term
// Pre: none
// Post: the coefficient, if an x^power Term exists, is return. If 
//      an x^power Term doesn't exist, 0 is returned

double Polynomial::coefficient(const int power) const {
    Term *thisPointer = head->next;
    while (thisPointer->power != power && thisPointer != head) thisPointer = thisPointer->next;
    if (thisPointer == head) return 0;
    return thisPointer->coeff;
}

// changeCoefficient: replaces the coefficient of the x^power term
// Pre: none
// Post: if an x^power Term exists, its coefficient will be changed to 
//      newCoefficient. If not, an x^power Term with that coefficient will be inserted

bool Polynomial::changeCoefficient(const double newCoefficient, const int power) {
    if (head == NULL) *this = Polynomial();
    Term *thisPointer = head->next;
    // either finds value to be changed or stops before head, indicating need for insertion
    while (thisPointer->power != power && thisPointer->next != head)
        thisPointer = thisPointer->next;
    // finds proper location for insertion if cycled through list
    if (thisPointer->next == head) {
        thisPointer = head;
        while (thisPointer->next->power > power) thisPointer = thisPointer->next;
        insert(thisPointer->next, newCoefficient, power);
    }
    else if (newCoefficient == 0) {
        remove(thisPointer);
    } else thisPointer->coeff = newCoefficient;
    return true;
}

// insert: inserts an x^power Term with coefficient newCoefficient into the
//      polynomial, directly just before the pos Term
// Pre: the function is passed a nonzero newCoefficient and a pos Term that
//      exists in the polynomial
// Post: returns true if new Term was successfully inserted, returns false if
//      pre conditions are not met

bool Polynomial::insert(Term* pos, const double newCoefficient, const int power) {
    if (newCoefficient == 0) return false;
    Term *thisPointer = head;
    while (thisPointer->next != pos && thisPointer->next != head)
        thisPointer = thisPointer->next;
    // returns false if pos Term is not found
    if (size > 0 && thisPointer->next == head && pos != head) return false;
    // creates new term using parameters
    Term *newTerm = new Term;
    newTerm->power = power;
    newTerm->coeff = newCoefficient;
    // redirects pointers of adjacent Terms to include newTerm in polynomial
    newTerm->next = thisPointer->next;
    newTerm->prev = thisPointer;
    thisPointer->next->prev = newTerm;
    thisPointer->next = newTerm;
    size++;
    return true;
}

// remove: removes pos Term from polynomial
// Pre: pos Term exists in polynomial
// Post: returns true if pos Term was successfuly removed, if not returns false

bool Polynomial::remove(Term* pos) {
    Term *thisPointer = head;
    // finds term before one to be deleted
    while (thisPointer->next != pos && thisPointer->next != head) thisPointer = thisPointer->next;
    //returns false if pos term is not found
    if (thisPointer->next == head) return false;
    // redirects pointers of adjacent Terms around pos, removes pos
    thisPointer->next = thisPointer->next->next;
    thisPointer->next->prev->prev = NULL;
    thisPointer->next->prev->next = NULL;
    delete thisPointer->next->prev;
    thisPointer->next->prev = thisPointer;
    size--;
    return true;
}

// Overloaded <<: prints Cn * x^n + Cn-1 * X^n-1 + ... C1 * X + C0

ostream& operator<<(ostream &output, const Polynomial& p) {
    Polynomial::Term *thisPointer = p.head->next;
    if (thisPointer != NULL) {
        while (thisPointer != p.head) {
            if (thisPointer->coeff < 0) output << "-";
            else if (thisPointer->prev != p.head) output << " + ";
            if (abs(thisPointer->coeff) != 1) output << abs(thisPointer->coeff);
            if (abs(thisPointer->power) == 1) output << "x";
            else if(thisPointer->power != 0) output << "x^" << thisPointer->power;
            thisPointer = thisPointer->next;
        }
    }
    return output;
}

// Overloaded operator+

Polynomial Polynomial::operator+(const Polynomial &in) const {
    Polynomial out = *this;
    out += in;
    return out;
}

// Overloaded operator-

Polynomial Polynomial::operator-(const Polynomial &in) const {
    Polynomial out = *this;
    out -= in;
    return out;
}

// Overloaded operator==

bool Polynomial::operator==(const Polynomial &in) const {
    Term *thisPointer = head->next;
    Term *inPointer = in.head->next;
    while (thisPointer != head || inPointer != in.head) {
        if (thisPointer->power != inPointer->power || 
            thisPointer->coeff != inPointer->coeff) return false;
        thisPointer = thisPointer->next;
        inPointer = inPointer->next;
    }
    return true;
}

// Overloaded operator!=

bool Polynomial::operator!=(const Polynomial &in) const {
    return !(*this == in);
}

// Overloaded operator=

Polynomial& Polynomial::operator=(const Polynomial &p) {
    if (*this != p) {
        this->~Polynomial();
        *this = p;
    }
    return *this;
}

// Overloaded operator+=

Polynomial& Polynomial::operator+=(const Polynomial &in) {
    Term *thisPointer = head;
    Term *inPointer = in.head;
    while (thisPointer->next != head || inPointer->next != in.head) {
        int power = inPointer->next->power;
        double coeff = inPointer->next->coeff;
        // if t > p, insert t in p
        if (power > thisPointer->next->power) {
            insert(thisPointer->next, coeff, power);
            thisPointer = thisPointer->next;
        }
        // if p power = t power, add 
        else if (power == thisPointer->next->power)
            thisPointer->next->coeff += coeff;
        if (thisPointer->next->coeff == 0) remove(thisPointer->next);
        // only advances t if p->next isn't larger than t->next
        if (inPointer->next->power <= thisPointer->next->power)
            thisPointer = thisPointer->next;
        inPointer = inPointer->next;
    }
    return *this;
}

// Overloaded operator-=

Polynomial& Polynomial::operator-=(const Polynomial &in) {
    Term *thisPointer = head;
    Term *inPointer = in.head;
    while (thisPointer->next != head || inPointer->next != in.head) {
        int power = inPointer->next->power;
        double coeff = inPointer->next->coeff;
        // if t > p, insert t in p
        if (power > thisPointer->next->power) {
            insert(thisPointer->next, -coeff, power);
            thisPointer = thisPointer->next;
        }
        // if p power = t power, subtract 
        else if (power == thisPointer->next->power)
            thisPointer->next->coeff -= coeff;
        if (thisPointer->next->coeff == 0) remove(thisPointer->next);
        // only advances t if p->next isn't larger than t->next
        if (inPointer->next->power <= thisPointer->next->power)
            thisPointer = thisPointer->next;
        inPointer = inPointer->next;
    }
    return *this;
}

@T.C. I take it I should change it to if(head == NULL) delete this; ? — Torin, Apr 13 '17 at 04:25
Absolutely not. Where did you even learn this stuff from? What kind of introductory material is teaching explicit destructor calls and `delete this`? — T.C., Apr 13 '17 at 04:31
Your copy constructor is also wrong. Some issues: don't compare this to anything in it, don't read `head` before assigning it a value (since it is initially has and undefined value), don't call the destructor, and if `pPointer` is NULL `size` has an unspecified value. — 1201ProgramAlarm, Apr 13 '17 at 04:38
@T.C. obviously I'm not being taught how to do it at all, never mind properly.... so in the case of deallocating the memory for an object in the copy constructor before copying the information, what would be the appropriate way to destruct the object? Do I just have to loop through the structure with remove() or is there a more efficient means of doing it? — Torin, Apr 13 '17 at 04:39
@Torin A constructor only creates an object where there was no object before. So no preexisting data is there, to deallocate or even to compare `!= NULL`. — Potatoswatter, Apr 13 '17 at 04:50
@1201ProgramAlarm I guess I had the operator precedence confused in my head, so I was thinking I had to deallocate and test for self assignment there as opposed to in operator=. — Torin, Apr 13 '17 at 05:14
@Potatoswatter tying into what I was saying to 1201ProgramAlarm I see that the testing should be done in operator=, not in the copy constructor — Torin, Apr 13 '17 at 05:15
@Torin `else if (this != &p) {` -- Why are you doing this in your copy constructor when you know that the object being constructed is brand new and could never equal to `&p`? Second, once you have the copy constructor working, make your job simple in the assignment operator `{Polynomial temp(p); std::swap(temp.head, head); std::swap(temp.size, size); return *this;}` -- Done. None of this calling the destructor explicitly stuff, checking for self assignment, etc. — PaulMcKenzie, Apr 13 '17 at 06:07
@Torin Also, your `-=` `+=` could be one liners. Example for `-=`: `{return Polynomial(*this) -= in;}` — PaulMcKenzie, Apr 13 '17 at 06:09
@PaulMcKenzie I was doing tests in my constructor, as I said in above comments, because I got the order of precedence mixed up. I did /not/ know that the copy constructor took effect AFTER the = operator. That was never clearly communicated to me. — Torin, Apr 13 '17 at 06:25
@Torin Yes, I meant the `-`, `+` etc. as one liners. Going back to your assignment operator, you could have avoided all of these issues with that simple implementation in my earlier comment. — PaulMcKenzie, Apr 13 '17 at 06:28
@PaulMcKenzie I forgot to add to my comment that since my teacher hasn't mentioned anything about the swap functions to us in class, I opted to hard copy instead of using swap, which is apparently and unfortunately the hard way — Torin, Apr 13 '17 at 06:32
@Torin You're not understanding -- the copy / swap **does** the copying and the "hard way". The solution is just calling the copy constructor in the first line, and using the destructor **you** wrote at the end of the function. All `swap` does is exchange the two values -- you could even write your own 3 line function to swap values -- it isn't a big deal. A student may have even came across it by seeing that they could leverage the copy constructor and destructor in this manner -- has nothing to do with "not learning yet". — PaulMcKenzie, Apr 13 '17 at 06:37
@Torin [Link to a recent answer](http://stackoverflow.com/questions/39950635/c-linked-list-assignment-operator/39950826#39950826). Please read carefully why this works, and that it really does not violate any "rules", as all it does is use the functions you already wrote, only in a smart way. — PaulMcKenzie, Apr 13 '17 at 06:47

score 3 · Answer 1 · edited May 23 '17 at 12:17

3

You implement the assign operator function as:

Polynomial& Polynomial::operator=(const Polynomial &p) {
    if (*this != p) {
        this->~Polynomial();
        *this = p;
    }
    return *this;
}

This will lead to infinite recursion if the the conditional in the if statement is true, which is the case for your use case.

It is also wrong and leads to undefined behavior.

After the line

        this->~Polynomial();

the object is dead. Any attempt to dereference this is cause for undefined behavior.

You may want to use the copy and swap idiom to implement the copy assignment operator.

edited May 23 '17 at 12:17

Community

1
1

answered Apr 13 '17 at 04:25

R Sahu

204,454
14
159
270

thanks for the advice. My teacher hasn't mentioned the swap methods in class yet, so I'm avoiding using methods outside of our curriculum and opting for hard copying instead – Torin Apr 13 '17 at 06:21
@Torin, you can certainly do that. You will have to deal with a bit of duplicate code. Read the answer in the link I added to my answer. It will help you understand which lines of code needs be duplicated in the copy constructor and the assignment operator. – R Sahu Apr 13 '17 at 15:03

Why would my overloaded operator+ make 'this' uninitialized on return when adding a variable to itself?

1 Answers1