-2

I am writing a program that should add, subtract and multiply polynomials. As well, overload the =, -, +, -=, +=, *, *= operators. I am required to use dynamic memory allocation. I was able to get the program running I can get input from the user send it to methods to do the calculations. The issue I am having is with the Add function returned value the beginning of the outputted polynomial is corrupted. To me it looks either like an address or garbage. I tried for many hours to figure out what's wrong; however I hit a wall and I am losing time and productivity. I know the math aspect of the code is not good, but for now I wanna make sure first that I have a correct input and output. I truly appreciate any guidance or advice I can get.

This is my code along with the sample output:

Polynomial.h

#ifndef POLYNOMIAL_H
#define POLYNOMIAL_H

class Polynomial
{

private:
    //A dynamic array of coefficients
    int * m_coefficientArray;

    //Holds the degree of a polynomial
    int m_degree;

    //Helper function creates a costume size array to hold operation (+ , - etc) results
    Polynomial(int size);

public:

    //<<<<<<< Methods >>>>>>>//

    //Default Constructor
    Polynomial();

    //Costume Constructor
    Polynomial(int * coefficientArray, int degree);

    //Copy Constructor to preserve data from being mutated
    Polynomial(const Polynomial & copy);

    //Destructor returns memory to the OS after no longer needed
    ~Polynomial();

    //Assingment op overload
    Polynomial & operator = (const Polynomial & rhs);

    //Addition
    Polynomial Add(Polynomial & other);

    //subtraction
    Polynomial Subtract(Polynomial & other);

    //Gets input from the user
    void Get_Data();

    //Display data
    void Display_Poly();

    // + operator overload
    Polynomial operator + (Polynomial & other);

};
#endif

Polynomial.cpp

#include "Polynomial.h"
#include <iostream>

using std::cout;
using std::cin;
using std::endl;

//Default Constructor
Polynomial::Polynomial()
{
    m_degree = 1;
    m_coefficientArray = new int[1];                            
    m_coefficientArray[0] = 0;  //First element of m_coefficientArray is 0.
}

//Costume Constructor
Polynomial::Polynomial(int * coefficientArray, int degree)                   
{
    m_degree = degree;

    m_coefficientArray = new int[degree];                       

    for (int i = 0; i < degree; i++)
    {
        m_coefficientArray[i] = coefficientArray[i];
    }
}

//Copy Constructor to preserve data from being mutated
Polynomial::Polynomial(const Polynomial & copy)
{
    m_degree = copy.m_degree;

    m_coefficientArray = new int[m_degree];          

    for (int i = 0; i < m_degree; i++)
    {
        m_coefficientArray[i] = copy.m_coefficientArray[i];
    }

}

//Destructor returns memory to the OS after no longer needed
Polynomial::~Polynomial()
{
    delete[] m_coefficientArray;
    m_degree = 0;
    m_coefficientArray = nullptr;
}

//Assignment operator overload
Polynomial &Polynomial::operator = (const Polynomial & rhs)
{
    if (this == &rhs)
    {
        return *this;
    }
    else
    {
        delete[] m_coefficientArray;

        m_coefficientArray = new int[rhs.m_degree];            

        m_degree = rhs.m_degree;

        for (int i = 0; i < m_degree; i++)
        {
            m_coefficientArray[i] = rhs.m_coefficientArray[i];
        }
    }
    return *this;
}

// Adds two polynomials together, then returns the sum
Polynomial Polynomial::Add(Polynomial & other)
{
    int size = (m_degree >= other.m_degree) ? m_degree : other.m_degree;

    Polynomial result(size);

        for (int i = 0; i < size; i++)
        {
            result.m_coefficientArray[i] = m_coefficientArray[i] + other.m_coefficientArray[i];
        }

        
        return result;
}

//Get users input
void Polynomial::Get_Data()
{


    
    cout << "Enter degree of polynomial: ";
    cin >> m_degree;    

    m_coefficientArray = new int[m_degree + 1]; //Array to hold the coeffs for polynomial     
    for (int i = m_degree; i >= 0; i--)
    {
        cout << "Enter coeffecient of x^" << i << ": ";
        cin >> m_coefficientArray[i];
    }
}

//Display and orginize program output
void Polynomial::Display_Poly()
{
    int i = 0;

    for (i = m_degree; i > 0; i--)
    {
        if (m_coefficientArray[i] == 0)
        {   //Prevents output like "0x^2, which is really just 0
            cout << "";
        }
        else if (i == 1)
        {   //Pevents output like "3x^1", the 1 is not necessary
            cout << m_coefficientArray[i] << "x" << " + ";
        }
        else if (m_coefficientArray[i] == 1)
        {   //Prevents output like "1x^2 + 1x", the 1 is not necessary
            cout << "x^" << i << " + ";
        }
        else //Normal, if others dont occur (coeff, variable, '^', power, '+')
        {
            cout << m_coefficientArray[i] << "x^" << i << " + ";
        }
    }
        cout << m_coefficientArray[i] << "\n";
}

//This constructor allows you to set the size of the array
Polynomial::Polynomial(int size)
{
    //check for invalid input
    if (size <= 0)
    {
        m_degree = 1;
    }//Make the default array of size 1 in this case
    else
    {
        m_degree = size;

        //Create an array of the given size
        m_coefficientArray = new int[m_degree + 1];

        
        //init everything to zero
        for (int i = 0; i <= m_degree; i++)
        {
            m_coefficientArray[i] = 0;
        }
    }
    
    
}

//Subtracts two polynomials, then returns the result
Polynomial Polynomial::Subtract(Polynomial & other)
{
    int size = (m_degree >= other.m_degree) ? m_degree : other.m_degree;
    Polynomial sub(size);

    for (int i = 0; i < size; i++)
    {
        sub.m_coefficientArray[i] = m_coefficientArray[i] - other.m_coefficientArray[i];
    }

    return sub;
}

// Addition operator overload
Polynomial Polynomial::operator + (Polynomial & other)
{

    return Add(other);
}

Main.cpp

#include "Polynomial.h"
#include <iostream>

using std::cout;
using std::cin;
using std::endl;

int main()
{
    Polynomial poly1;
    Polynomial poly2;
    Polynomial poly3;

    //input
    poly1.Get_Data();
    poly2.Get_Data();

    //output
    cout << "First Polynomial:" << endl;
    poly1.Display_Poly();

    cout << "Second Polynomial:" << endl;
    poly2.Display_Poly();

    cout << "Addition result " << endl;
    poly2.Add(poly1).Display_Poly();

    cout << "Subtraction Result " << endl;
    poly1.Subtract(poly2).Display_Poly();

    cout << "+ Operator Test " << endl;
    poly3 = poly1 + poly2;

    return 0;
}

Program Output:

Enter degree of polynomial: 1

Enter coeffecient of x^1: 2

Enter coeffecient of x^0: 3

Enter degree of polynomial: 2

Enter coeffecient of x^2: 1

Enter coeffecient of x^1: 2

Enter coeffecient of x^0: 3

First Polynomial:

2x + 3

Second Polynomial:

x^2 + 2x + 3

Addition result

-33686019x^2 + 4x + 6

Subtraction Result

-33686019x^2 + 0

Operator + Test

-33686019x^2 + 4x + 6

Press any key to continue . . .

Community
  • 1
  • 1
ashumrani
  • 46
  • 7
  • 2
    Minimal example, please. – SergeyA Mar 03 '16 at 21:39
  • "*I am required to use dynamic memory allocation*" -- What a bummer. You don't know now if the problem is the maintenance of a dynamic array, or that you really have a problem with adding. – PaulMcKenzie Mar 03 '16 at 21:43
  • 1
    One observation -- Your assignment operator (`operator=`) is flawed. It deletes the memory before calling `new[]`. What if `new []` throws an exception? You would have corrupted your object. Look at the copy / swap to implement assignment easily and safely: http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom. It also doesn't make sense to set pointers to NULL and assign members with 0 in the destructor. The object is going to be destroyed anyway. – PaulMcKenzie Mar 03 '16 at 21:47
  • even if you are required to use dynamic memory allocation, you should use `std::vector` for the coefficients. It dynamically allocates memory for you and you can use it with std algorithms. – 463035818_is_not_a_number Mar 03 '16 at 21:50
  • Suggestion. Forget the dynamic array for now and use a vector. Get your code working without the memory management distraction and put the dynamic array back in when you have a solid foundation. One problem at a time saves time. – user4581301 Mar 03 '16 at 21:51
  • @user4581301 That is a good idea. Make sure that the actual goal (manipulating polynomials) actually works first. Then once that's working, *then* go back and do this tedious stuff of home-made dynamic array maintenance. – PaulMcKenzie Mar 03 '16 at 21:56
  • 1
    @OP. You should really overload `+=`, `*=`, and `-=` first. Then when you get those working, you implement `+`, `*`, and `-` in terms of the composite operators. – PaulMcKenzie Mar 03 '16 at 21:58
  • Thanks Paul, tobi303, user and Sergey I will use your advice. I truly appreciate your help and time. – ashumrani Mar 03 '16 at 22:33

3 Answers3

1

You have two significant errors.


Error 1:

As others have mentioned, you have an error if you're adding two Polynomials, and one Polynomial is of a different degree than the other Polynomial:


Error 2:

The other error is that in your Polynomial(int) constructor, you failed to set m_coefficientArray to nullptr if the size is 0. Thus if you were to do this:

{
   Polynomial p(0);
}

This one line would cause undefined behavior on destruction, since your destructor will attempt to call delete on an uninitialized pointer. Even though it is private, you should ensure that all of your constructors won't cause unnecessary issues like this.


There are other issues with your code which you should address. One is the assignment operator could easily have been written this way:

Polynomial &Polynomial::operator = (const Polynomial & rhs)
{
    // make a temp copy of the passed-in object
    Polynomial temp(rhs);

    // swap out temp's internals with our internals 
    std::swap(temp.m_coefficientArray, m_coefficientArray);
    std::swap(temp.m_degree, m_degree);

    // return ourselves
    return *this;
}  // temp dies off with the old data here

This is the copy / swap idiom. Your current implementation of the assignment operator will not work correctly if new[] throws an exception, since you would have already deallocated the memory. Using copy / swap, this can never happen since if something goes wrong in creating temp, the original object is not harmed in any way.

Community
  • 1
  • 1
PaulMcKenzie
  • 31,493
  • 4
  • 19
  • 38
0

You are reading from memory you don't own in your Add and Subtract methods when this->m_degree != other.m_degree. Consider the case when this->m_degree is 2 and other.m_degree is 3. You will attempt to read from this->m_coefficientArray[3], but that is outside the bounds of the array you allocated.

Miles Budnek
  • 19,769
  • 2
  • 26
  • 41
0
int size = (m_degree >= other.m_degree) ? m_degree : other.m_degree;

for (int i = 0; i < size; i++)
    ... m_coefficientArray[i] + other.m_coefficientArray[i]

You set size to be the maximum of the two sizes, but you iterate through both arrays size times. If the arrays didn't have equal length, you're going to overshoot the bounds of one of them.

Since you need to iterate over each element in both arrays, you might be better off looping over each one separately.

for i from 0 to size
    result[i] += arr[i]
for i from 0 to other.size
    result[i] += other.arr[i]

You could alternatively loop to the minimum size of either array, then iterate over the remainder of the other one. It would probably be faster, but it would require more writing.

Weak to Enuma Elish
  • 4,531
  • 3
  • 22
  • 36