overloading * operator

[gnarlyskim]

Having issues with this program. I've been trying to identify all the areas where data is created, then tried to destroy it likewise after using it. I've put a lot of delete[] coef (coef is a pointer double array to hold coefficients). The program will run half the time, then crash half of the time. The algorithms are correct for what I need to do; however, I just can't wrap my head around the crash issue, nor identify the memory issue. Any help is greatly appreciated!

//multiply function used within the overloading, the multiplication takes one polynomial as constant, then breaks apart the second part one term at a time and uses a private constructor to shift the first polynomial by each term's degree and scale it by the coefficient of that term.
void Polynomial::mult(const Polynomial& pol)
{
    Polynomial r;
    r.degree=degree+pol.degree;
    delete[] r.coef;
    r.coef=new double[r.degree+1];
    for(int i=r.degree;i>=0;i--)
        r.coef[i]=0.0;
    Polynomial copy;
    delete[] copy.coef;
    copy=*this;

    for(int i=pol.degree;i>=0;i--)
    {
        Polynomial p1(copy,pol.coef[i],i);
        r.add(p1);
        p1.~Polynomial();
    }

    delete[] coef;
    *this=r;
    r.~Polynomial();
    copy.~Polynomial();
}

//overloaded operator to multiply two polynomial objects, while keeping the original objects the same (const).
//called as a friend function
Polynomial operator*(const Polynomial& p1, const Polynomial& p2)
{
    Polynomial pol1(p1);
    Polynomial pol2(p2);

    pol1.mult(pol2);
    return pol1;

    pol1.~Polynomial();
    pol2.~Polynomial();
}

//how it's called in main
cout<<"("<<p1<<")*("<<p2<<")="<<p1*p2<<endl;

//how it's outputted
ostream& operator<<(ostream& stream, const Polynomial& pol)
{
//outputting using "stream<<output"
    return stream;
}

[pecet]

I can see lot of pointers and new / delete stuff in your code. So the question is if You overloaded operator=. Cause maybe shallow copy causes troubles?

[MrSpigot]

Without seeing the rest of this class (e.g. constructor/destructor) it's not possible to give a full answer to your problem. However, explicitly calling destructors on your local variables is a bad idea. The destructor will be called again when it goes out of scope.
The point at which it crashes would also be relevant.

[gnarlyskim]

Sorry for the lack of information. Here's a little more that is also relevant.

As for when it crashes, it varies on occasion. I've added cout's to the overloaded definition of * before returning a value, and sometimes it will display the coef array, and other times it will crash before that. I'm 90% sure that the algorithms are correct, I'm just especially new to the OOP concepts so memory allocation/delocation is spotty for me. Thanks for the input so far.

//class
class Polynomial
{
private:
    double* coef;
    int degree;
    Polynomial(const Polynomial& base,double scalar,int mdegree);
    void reset(); // sets degree to zero, array one elem, and the coefficient to 0.0
    void checkandreduce(); // check if the degree has decreased after addition and reduce the polynomial

public:
    Polynomial(){coef=new double[1];degree=0;};
    Polynomial(const Polynomial& pol);
    ~Polynomial(){delete [] coef;}
    int getDegree(){cin>>degree; return degree;};
    bool check();
    void add(const Polynomial& p2);
    void multc(double factor);
    void subtract(const Polynomial& p2);
    void mult(const Polynomial& pol);
    void div(const Polynomial& divisor, Polynomial& quotient, Polynomial& remainder);
    Polynomial& operator=(const Polynomial& pol);

    friend Polynomial operator+(const Polynomial& p1, const Polynomial& p2);
    friend Polynomial operator-(const Polynomial& p1, const Polynomial& p2);
    friend Polynomial operator*(const Polynomial& p1, const Polynomial& p2);
    friend Polynomial operator/(const Polynomial& p1, const Polynomial& p2);
    friend Polynomial operator%(const Polynomial& p1, const Polynomial& p2);
    friend istream& operator>>(istream& stream, Polynomial& pol);
    friend ostream& operator<<(ostream& stream, const Polynomial& pol);

};

//copy constructor
Polynomial::Polynomial(const Polynomial& pol)
{
        degree=pol.degree;
        if(pol.coef)
        {
            coef=new double[degree+1];
            for(int i=degree;i>=0;i--)
                coef[i]=pol.coef[i];
        }
        else coef=0;
}

//overloaded operator=
Polynomial& Polynomial::operator=(const Polynomial& pol)
{
    if(this == &pol)
        return *this;

    delete [] coef;
    degree=pol.degree;

    if(coef != pol.coef)
    {
        delete[] coef;
        coef=new double[degree+1];
        for(int i=degree;i>=0;i--)
            coef[i]=0.0;
        for(int i=degree;i>=0;i--)
            coef[i]=pol.coef[i];
    }
    else coef=0;
    return *this;
}

//constructor that creates a polynomial of the first polynomial multiplied by each consecutive term
Polynomial::Polynomial(const Polynomial& base,double scalar,int mdegree)
{
    degree=base.degree+mdegree;
    delete[] coef;
    coef=new double[base.degree+mdegree+1];
    for(int i=degree;i>=0;i--)
        coef[i]=0.0;

    for(int i=base.degree;i>=0;i--)
        coef[i+mdegree]=base.coef[i];
    for(int i=mdegree-1;i>=0;i--)
        coef[i]=0.0;
    multc(scalar);
}

[Banfa]

In your first code listing you are explicitly calling the destructors on many of your automatic scope objects, lines 18, 24, 37, 38, but that is completely unnecessary. The compiler automatically adds destructor calls for these objects so you destruct them twice. Since that is likely to involve a delete call it is a receipe for disaster.

In your most recent code listing you delete coefs without zeroing it, test the pointer value of coefs and then deleteit again line 53 - 58.

You should not be deleting an object more than once. Anything else is undefined behaviour (or a memory leak if you don't delete it at all).

Line 56, how can this be anything other than true? If it is false you have a problem because you have 2 polynomial objects using the same data buffer.

On the whole I think your entire class would be simpler if you just used vector<double> coef rather than double* coefsince it would perform the memory management for you.

And finally this for(int i=base.degree;i>=0;i--) is quite a strange way to construct a for loop that is not required to go backwards (minor note and would be an infinite loop if you used unsigned variables).

Normall most people code for loops as for(int i=0;i<base.degree;i++) unless there is a specific need to go backwards through the array. if you used a vector you could just use a reverse iterator if required.

[gnarlyskim]

MrSpigot and Banfa, I deleted all of my explicitly called destructors and everything within the program that I've posted to here works perfectly. I really appreciate the time for y'all to look through and help me out.

Banfa, unfortunately simplicity is a word never associated with undergrad "weed-out" courses in school. We aren't covering vectors in the course, but in the past I've had that suggestion from a few other people on this forum, so I'll have to do some research on my own time after the course ends. As far as the for loops, I've been working on this code for a few weeks (over the course of several projects) and I initially started for loops by going through them backwards, so I implemented every loop I would use in a backwards manner. For the sake of keeping what I know works constant and "staying on the same page" conceptually with the rest of the code, I'm just going to finish this project as-is. In the future, moving forwards is always better than going backwards so I'll remember that ha. Thanks a ton though!

[gnarlyskim]

Well it turned out that I overused the delete[] coef's to an extreme and it hurt me(contrary to what I thought). My code now runs completely in sync with the solution.exe, what a great way to finish the final project of my C++ course. Thanks to all who have helped me in these posts! Time for some drinks...after all it is Friday right?!