0

I've been having problems with my homework, regarding making a polynomial class and overloading some operators to work according to the class. I've done most of the thing, but I seem to be having a problem with my destructor, or my functions.

The problem is, according to my debugging(if I did it right, which I think I did), the returning value of my + operator function get's destructed twice, when used with my copy constructor, in something like this :

//polynomials p1 and p2 are declared and given values beforehand
Polynomial p5=Polynomial();
p5=p1+p2;

This results in a heap corruption error.

Here's my header code:

#ifndef POLYNOMIAL_H_
#define POLYNOMIAL_H_
#include <iostream>

using namespace std;

class Polynomial
{
public:
    Polynomial();
    Polynomial(int);
    ~Polynomial();
    Polynomial(const Polynomial &);
    int getOrder() const; 
    double getCoefficient(int) const; 
    void setOrder(int);
    void setCoefficient(int,double);
    const Polynomial &operator=(const Polynomial &);
    const bool &operator==(const Polynomial &);
    const double &operator()(double &);
    const bool &operator!=(const Polynomial &);

    friend Polynomial operator+(const Polynomial &poly1, const Polynomial &poly2);
    friend Polynomial &operator+=(Polynomial &poly1,const  Polynomial &poly2);
    friend Polynomial operator-(const Polynomial &poly1, const Polynomial &poly2);
    friend Polynomial &operator-=( Polynomial &poly1,const  Polynomial &poly2);
    friend Polynomial operator*(Polynomial poly1,double num);
private:
    int order;
    double *coefficient;
};


#endif

and here's my overloaded + function, it's not pretty, but my problem is not the calculations, it's the memory. I declared it as a friend function in the class, and by my homework rules, i need to implement it in the main.cpp file as a free function, and not as a member function. 

   Polynomial operator+(const Polynomial &poly1, const Polynomial &poly2) //the overloaded +operator. makes the order of the result the bigger order and adds the coefficients for all the orders. returns the result. 
{
    Polynomial result;


    if(poly1.order >= poly2.order)
    {
        result.setOrder(poly1.order);
        for(int i=poly1.order;i>poly2.order;i--)
        {
            result.setCoefficient(poly1.order-i, poly1.coefficient[poly1.order-i]);
        }
        for (int i =poly2.getOrder(); i>=0;i--)
        {
            result.setCoefficient(poly1.order-i,poly1.coefficient[poly1.order-i]+poly2.coefficient[poly2.order-i]);
        }
    }
    else 
    {
        result.setOrder(poly2.order);

        for(int i=poly2.order;i>poly1.order;i--)
        {
            result.setCoefficient(poly2.order-i, poly2.coefficient[poly2.order-i]);
        }
        for (int i =poly1.order; i>=0;i--)
        {
            result.setCoefficient(poly2.order-i,poly1.coefficient[poly1.order-i]+poly2.coefficient[poly2.order-i]);
        }
    }
    return result;
}

We were also required to overload the = operator, and here's that function if it's needed.

    const Polynomial &Polynomial::operator=(const Polynomial &poly)
{
    if(this!=&poly)
    {
        if(order==poly.order)
        {
            for(int i=0;i<=order;i++)
            {
                coefficient[i]=poly.coefficient[i];
            }
        }
        else 
        {
            coefficient=new double[poly.order];
            order=poly.order;
            for(int i=0;i<=order;i++)
            {
                coefficient[i]=poly.coefficient[i];
            }
        }
    }
    return *this;
}

Please keep in mind that I'm really a beginner in coding and c++, and I appreciate any help you give.

EDIT: Adding the deep copy constructor. 

 Polynomial::Polynomial(const Polynomial &copy) //deep copy constructor
{
    order=copy.order;
    coefficient=new double[copy.order];
    for (int i=0;i<=order;i++)
    {
        coefficient[i]=copy.coefficient[i];
    }
}
Ardıç Sönmez
  • 13
  • 5
  • can you provide code for Polynomial(const Polynomial &); as well? – alexrider Apr 19 '13 at 12:19
  • added the code for Polynomial(const Polynomial &) – Ardıç Sönmez Apr 19 '13 at 12:24
  • Your copy assignment operator is leaking memory. You never delete the old coefficient array before creating a new one. Also, have you considered just using std::vector here? It would get rid of the memory leak and any implementation worth anything would have caught the out-of-bounds access in debug mode too. – Sebastian Redl Apr 19 '13 at 12:28
  • So do I need to delete[] coefficient before I create the new coefficient? I would LOVE to use the vector here, but my homework assignment states that I need to do this using dynamic arrays. Thanks for the answer. – Ardıç Sönmez Apr 19 '13 at 12:31
  • @SebastianRedl, but memory leak is hardly a reason for heap corruption. I would guess that the problem IS around the coefficient, but somewhere else (in on of the other functions), maybe setCoefficient? – W.B. Apr 19 '13 at 12:34
  • through my debugging, I saw I got the heap corruption error when the destructor was called, could the problem's reason still be another function? – Ardıç Sönmez Apr 19 '13 at 12:37
  • unary operators like `+` or `==` is better to implement as members when binary operators like `+=` is better to implement as free (optionally friend) functions. – Andriy Tylychko Apr 19 '13 at 12:40
  • Actually, your copy constructor seems to be faulty. You allocate an array of doubles and in the for loop your index go out of bounds, when `i == order`. – W.B. Apr 19 '13 at 12:42
  • I fixed that, but I'm still having the heap corruption error. – Ardıç Sönmez Apr 19 '13 at 12:44
  • well, not anymore. thanks a lot to those who helped me, i figured it all out. – Ardıç Sönmez Apr 19 '13 at 12:52

2 Answers2

1

You allocate memory for poly.order doubles, but in your for loop last index would be poly.order while it should poly.order-1 changing <= to < will solve that.

   coefficient=new double[poly.order];//This is memory leak previous coefficient needs to be deleted
    order=poly.order;
    for(int i=0;i<=order;i++)//<= will lead to overflow and heap corruption
    {
        coefficient[i]=poly.coefficient[i];
    }
alexrider
  • 4,449
  • 1
  • 17
  • 27
0

In your copy constructor and your overloaded operator= you are iterating one extra time when you copy the cofficient array:

  coefficient=new double[copy.order];
  for (int i=0;i<=order;i++)
                ^^

That should be be <, you have allocated order space but you are iterating order+1 times. You also need to call delete [] the old coefficient array as well. Normally the safe way to do this is allocate to a temporary first and modify that and if all those potentially unsafe operations are successful then you can delete and assign the temporary to coefficient. You can see this SO post on Rule of Three for a solid explanation on what you need to do when using a copy constructor and assignment.

Community
  • 1
  • 1
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
  • ah ah yeah i got it. i have to make all the new allocations copy.order+1, since that's the amount of coefficients i need to have. Thanks a lot for the answers. – Ardıç Sönmez Apr 19 '13 at 12:28
  • do i need to delete anything in the copy constructor, since I'm creating it from scratch, or do i need to call delete[] for the = operator only? – Ardıç Sönmez Apr 19 '13 at 12:35
  • @ArdıçSönmez I explained it a little bit in my updated post but you should see the `Rule of Three` post I added this will explain in detail what you need to do. – Shafik Yaghmour Apr 19 '13 at 12:38
  • Thanks a lot, as I'm still new, I'm having problems getting the core principles in my head. – Ardıç Sönmez Apr 19 '13 at 12:40