Copy Constructor for Class Linked List not working

Question

My copy constructor is referring to another address instead of copying the values, can you guys please find the problem for me?

This is the copy constructor:

Poly::Poly(const Poly& copyList)
{

    // copy the first term
    power = copyList.power;
    coef = copyList.coef;

    if (copyList.nextTerm == NULL)
    {
        nextTerm = NULL;
    }
    else
    {
        // copy the next term
        PolyPtr trackCopy = copyList.nextTerm;
        nextTerm = new Poly;
        PolyPtr trackOriginal = nextTerm;
        PolyPtr newTerm;

        while (trackCopy != NULL)
        {
            newTerm = new Poly;
            newTerm->coef = trackCopy->coef;
            newTerm->power = trackCopy->power;
            trackOriginal->nextTerm = newTerm;
            trackOriginal = newTerm;

            trackCopy = trackCopy->nextTerm;

        }   
    }

}

copyList here is a class with private member variables as coef, power and a nextTerm that refers to the next node of the list

Here are is the interface of Poly:

class Poly
{
    public:
        // constructors
        Poly();
        Poly(int constant);
        Poly(int coef, int power);
        Poly(const Poly& copyList);

        // set functions
        void setPower(int number);
        void setCoef(int number);
        void setNextTerm(Poly* link);

        // member function to compute
        int evaluate(int x);


    private:
        int power;
        int coef;
        Poly* nextTerm;

};
typedef Poly* PolyPtr;

An example is here:

int main()
{
    PolyPtr polyNomial = new Poly(2, 3);
    PolyPtr nextTerm = new Poly(5, 8);

    polyNomial->setNextTerm(nextTerm);

    PolyPtr copyTerm(polyNomial);

    PolyPtr newTerm = new Poly(1, 2);
    copyTerm->setNextTerm(newTerm);

    cout << copyTerm->evaluate(3) << endl;

    cout << polyNomial->evaluate(3) << endl;


    return 0;
}

Here I expected the evaluation of the copyTerm and polyNomial to be different but they are the same

Make sure that constructors do initialization, e.g. `Poly::Poly() : nextTerm (nullptr), trackCopy (nullptr)` — Swift - Friday Pie, Dec 30 '20 at 07:27
Make sure you implement the [rule of three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). A [mre] would be helpful — Alan Birtles, Dec 30 '20 at 08:04
"_can you guys please find the problem for me_" - How does the problem manifest itself? — Ted Lyngmo, Dec 30 '20 at 08:05
Too complicated. The copy constructor should be while more nodes in source list, copy current source node, add to copy to new list, advance to next node in source list. Everything else is just chaff for other bugs to hide in. — user4581301, Dec 30 '20 at 08:11
Say you have a `Poly` of length two, so a single non-null `nextTerm` pointer. How many times should you call `new`? How many times are you in fact calling `new`? — n. m. could be an AI, Dec 30 '20 at 08:28

Ted Lyngmo · Accepted Answer · 2020-12-30T10:38:59.923

Your copy constructor has a bug. I added this friend function to display the linked list and a small program to create Polys and to copy them:

std::ostream& operator<<(std::ostream& os, const Poly& p) {
    const Poly* n = &p;
    os << '{';
    do {
        os << "addr:" << n << ' ' << n->power << ',' << n->coef << ' ';
        n = n->nextTerm;
    } while(n);
    return os << '}';
}

int main() {
    Poly p(1,2);
    p.setNextTerm(new Poly(3,4));

    std::cout << "orig: " << p << '\n'; // display the original
    Poly c(p);                          // copy construction
    std::cout << "copy: " << c << '\n'; // display the copy
}

Note the extra Poly with zeroes inserted in the copy:

orig: {addr:0x7ffcf2af8cd0 2,1 addr:0x1447eb0 4,3 }
copy: {addr:0x7ffcf2af8ce0 2,1 addr:0x1448ee0 0,0 addr:0x1448f00 4,3 }

If you simplify the copy constructor like this:

Poly::Poly(const Poly& rhs) : power(rhs.power), coef(rhs.coef), nextTerm(nullptr) {
    for(Poly *prev = this, *next = rhs.nextTerm; next; next = next->nextTerm) {
        prev->nextTerm = new Poly(next->coef, next->power);
        prev = prev->nextTerm;
    }
}

The output will be something like this instead:

orig: {addr:0x7ffdfe4b2eb0 2,1 addr:0x1f15eb0 4,3 }
copy: {addr:0x7ffdfe4b2ec0 2,1 addr:0x1f16ee0 4,3 }

@swittuh You're welcome! I should probably mention that I think your class is missing a destructor too: `~Poly() { delete nextTerm; }` seems to be missing. — Ted Lyngmo, Dec 30 '20 at 13:54
Yes I was going to add that after I finished the copy constructor, which I got stuck on lolll — swittuth, Dec 30 '20 at 15:20

Copy Constructor for Class Linked List not working

1 Answers1