0

I am trying to create a Polynomial class, whose representation is a dynamically allocated 2D array (stored in data member int** arrPtr). The assignment operator for the class is problematic, as the delete operator throws the exception only when I use the assignment operator.

I set up some print statements in my code and learned that the coefficients and exponents of the argument polynomial are successfully copied into the target polynomial despite the error.

Here is the constructor, destructor, and assignment operator definitions (I'm also providing getCoefficient() since I used it in the assignment operator definition):

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

    Polynomial::Polynomial(int degree) : degree{ degree } {
        arrPtr = new int* [2];

        for (int i{ 0 }; i < 2; i++) { 
            arrPtr[i] = new int[degree + 1]{}; 
        }

        for (int i{ 0 }; i <= degree; i++) { 
            arrPtr[1][i] = i;
        }
    }

    Polynomial::~Polynomial() {
        for (int i{ 0 }; i < 2; i++) {
            delete[] arrPtr[i];
        }

        delete[] arrPtr; 
    }

    Polynomial& Polynomial::operator=(Polynomial polynom) {

        for (int i{ 0 }; i < 2; i++) { 
            delete[] arrPtr[i];
            arrPtr[i] = new int[polynom.degree + 1];
        }

        degree = polynom.degree;

        for (int i{ 0 }; i <= degree; i++) { 
            arrPtr[0][i] = polynom.getCoefficient(i);
            arrPtr[1][i] = i;
        }

        return *this;
    }

    int Polynomial::getCoefficient(int term) {
         if (term <= degree) {
             return arrPtr[0][term];
         }

         else {
             throw std::out_of_range("Term exceeds the degree of the polynomial.");
         }
    }

EDIT: Thanks for the vivid response, everyone. This was a newbie mistake, not defining a copy constructor, which I did, and everything works now. I guess I won't be forgetting the Rule of Three anytime soon.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Raul Abdullayev
  • 26
  • 1
  • 5
  • 5
    You need to write a copy constructor as well. Most likely this is a duplicate of: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – NathanOliver Jun 01 '20 at 14:43
  • Most likely you must implement a copy constructor too, however because the shown code fails to meet all requirements for a [mre], as explained in the [help], it is not possible for anyone to tell you, with 100% certainty, the reason for the crash. – Sam Varshavchik Jun 01 '20 at 14:43
  • 2
    Do you have copy constructor? If yes, please show it, if not, [What is the Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Yksisarvinen Jun 01 '20 at 14:43
  • 5
    Unless your assignment/exercise is about manual memory handling, then don't do it. Use `std::vector` everywhere instead. – Some programmer dude Jun 01 '20 at 14:45
  • 2
    You are much better off using two separate `std::vector` rather than reinventing the wheel and failing at it. – Tanveer Badar Jun 01 '20 at 14:45
  • I'd advise getting rid of the manual memory management (explicit `new`, `new[]`, `delete`, `delete[]`) entirely and instead use containers like `std::array` & `std::vector` as well as smart pointers like `std::unique_ptr`, `std::shared_ptr` & `std::weak_ptr`. That will make your life a lot simpler. – Jesper Juhl Jun 01 '20 at 14:46
  • One more thing: What's the point of second array? You store indices in there, but they are always just indices. You make sure that `arrPtr[1][i]` is equal to `i` (for every `i` in range), why not use `i` instead and have a simpler one-dimensional array? – Yksisarvinen Jun 01 '20 at 14:49
  • Also, your assignment operator is questionable. It is not exception safe, but worse, will have serious problems if you assign a variable to itself (since it'll delete its own data first, and then copy from "other" but in this case,"other" is itself, so it reads from its own deleted member.) Also, pay attention to what everyone said about the copy constructor: without it, the compiler will generate a default one that leaves objects pointing to the same data (copying pointers, shallowly), and once the first one is destroyed, the other will point to deleted data. – Chris Uzdavinis Jun 01 '20 at 15:28
  • That assignment operation need not be written in the way you wrote it. If your copy constructor works, your destructor works, then the assignment operator is just two `swap` calls and a return. Not only is it much shorter, it is guaranteed to work, won't have the exception-safety issues, etc. `Polynomial& Polynomial::operator=(Polynomial polynom) { std::swap(degree, polynom.degree); std::swap(arrPtr, polynom.arrPtr); return *this;}` – PaulMcKenzie Jun 01 '20 at 15:30

0 Answers0