overloading addition operator

Question

I have a linked list that is represented as

struct term{
    double coef;
    unsigned deg;
    struct term * next;
    };

then i have a polynomial class

class Polynomial{
public:
    Polynomial & operator+ (const Polynomial & ) const;
private:
    term *ptr

and i am trying to do an addition overloaded operator, but what i tried just give me some random part of the polynomial that is in the middle.

Polynomial & Polynomial::operator+(const Polynomial & poly) const{
    Polynomial p2 = *this;
    term * temp = (*this).ptr;
    while(temp->next != NULL){
        temp = temp->next;
    }
    temp->next = poly.ptr;
    return p2;
}

and also when i have 2 polynomials, one is a copy of another just then added one more term, then when i try to use the addition operator, the first polynomial is bigger, like the second polynomial is added to it.

Unless you're assigned to do otherwise, I strongly advise using a standard container of terms (`std::vector`, `std::list`, etc.), and doing away with the links. The results would be a near-trivial implementation for the operator you're attempting to conceive, especially for combining like-terms (which I think this is ultimately going toward by the looks of it). — WhozCraig, Nov 27 '12 at 06:33

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

2

You're returning a temp by reference, which is undefined behavior. Return a Polynomial instead. I.e.

Polynomial & operator+ (const Polynomial & ) const;

should be

Polynomial operator+ (const Polynomial & ) const;

You're also missing a copy constructor and copy assignment operator.

edited May 23 '17 at 12:18

Community

1
1

answered Nov 27 '12 at 06:23

Luchian Grigore

253,575
64
457
625

1

@JackF then this is only part of the problem. Make no mistake, it is a problem. – Luchian Grigore Nov 27 '12 at 06:27
@JackF it is both a *big* problem and a dreadfully difficult one to catch if you do it accidentally, as some implementations appear to just "work" but are not defined to none-the-less. – WhozCraig Nov 27 '12 at 06:29
@JackF show them and their implementation. Actually, no. Reduce the code to the minimum that exibits the problem and post it on ideone.com. – Luchian Grigore Nov 27 '12 at 06:32
@JackF Update your code on your ideone post to qualify the `operator +()`. You forgot the Polynomial:: scope in the declaration, which is leading to your build errors. – WhozCraig Nov 27 '12 at 06:46
@JackF you didn't narrow it down. I'm not gonna do your debugging for you. You have to help us help you, not expect us to solve your problem with no input from you. – Luchian Grigore Nov 27 '12 at 06:51
@LuchianGrigore I didn't narrow it down? I just have problem with the + operator, and you wanted to see the = and copy constructor – Jack F Nov 27 '12 at 06:52
@JackF I fixed the code to compile - http://ideone.com/YOF8FM - what's wrong with it? – Luchian Grigore Nov 27 '12 at 06:53
@JackF okay I see what's wrong & pretty clear you didn't try debugging. Here's the last tip - learn to debug & do it. – Luchian Grigore Nov 27 '12 at 06:56
@LuchianGrigore can you delete the compiled version of my program? – Jack F Nov 27 '12 at 07:35

WhozCraig · Accepted Answer · 2012-11-27T07:56:18.130

You're operator+() is seriously whacked. Consider the idea of "term" ownership. Each polynomial has a linked list of terms. It owns this list (at least it better). Now consider this brief analysis of your operator +():

Polynomial Polynomial::operator+(const Polynomial & poly) const 
{
    // hopefully creates a deep copy of *this
    Polynomial p2 = *this;

    // walk *our* terms (not the copy in p2??) to find the end.
    term * temp = (*this).ptr;
    while(temp->next != NULL)
        temp = temp->next;

    // once we find the end, we then *LINK* the params term-list
    //  which *they* are **supposed** to own, to our list. (and 
    //  p2 is still out there with a copy of our original content).
    temp->next = poly.ptr;

    // now we return p2, still holding a copy of our former self,
    // and we now have a cross-linked term list between us and the 
    // parameter poly
    return p2;
}

I sincerely hope it is evident what is wrong with that. For this to work correctly, your operator should be returning a by-val, (which it is, hooray!), and manufacture that thing correctly:

Make a copy (lets call it p2) of *this (you have that)
Find the end of the term list owned by p2
Duplicate all terms in the rhs parameter of operator +(const Polynomial* rhs), linking the copies one-by-one to the tail of p2's term list. Note: the tail will move with each new term linked.
Return p2 by val. If your copy-ctors and destructors are doing their job, everything should come out fine. When done, both *this, and rhs should be untouched.

Thats about the extent I can offer. Good luck.

PS: For extra-credit-bonus-round, sort your terms in your list as you insert them. This will get you one step closer to a like-degree merge, which will be the backbone of operator +=() and greatly assist your operator +(). The latter literally degenerates to Polynomial p2 = *this; p2 += poly; return p2;

overloading addition operator

2 Answers2