Assignment Operator overloading and invalid memory

Question

I'm trying to overload an assignment operator like this -

polynom polynom::operator=(const polynom& p)
{
    this->grad = p.grad;
    this->arr = new double[grad+1];
    for(int i = 0; i <= p.grad; i++)
    {
        this->arr[i] = p.arr[i];
    }
    return *this;
}

here's the .h file:

    #pragma once
using namespace std;
#include <iostream>
class polynom
{
public:
    polynom(int grad, double* arr);
    polynom(int grad);
    polynom(polynom& p);
    polynom();
    ~polynom(void);
    polynom operator=(const polynom& p);
    friend ostream& operator<<(ostream& os, const polynom& p);
private:
    int grad;
    double* arr;
};

.cpp:

#include "polynom.h"


polynom::polynom(int grad, double* arr)
{
    this->grad = grad;
    this->arr = new double[grad+1];
    for(int i = 0; i<=grad;i++)
    {
        this->arr[i] = arr[i];
    }
}

polynom::polynom(int grad)
{
    this->grad = grad;
    this->arr = new double[grad+1];
}

polynom::polynom()
{
    arr = NULL;
}

polynom::polynom(polynom& p)
{
}

polynom::~polynom()
{
    delete[] arr;
}

polynom polynom::operator=(const polynom& p)
{
    this->grad = p.grad;
    this->arr = new double[grad+1];
    for(int i = 0; i <= p.grad; i++)
    {
        this->arr[i] = p.arr[i];
    }
    return *this;
}

ostream& operator<<(ostream& os, const polynom& p)
{
    for(int i = 0; i <= p.grad; i++)
    {
        if(i == 0)
            os  << p.arr[i];
        else
        {
            if(p.arr[i]>=0)
                os << " + ";
            os << p.arr[i] << "x^"<<i;
        }
    }
    return os;
}

main:

#include <iostream>
#include "polynom.h"
using namespace std;

void main()
{
    double arr1[] = {1,2,3};
    double arr2[] = {5,9};
    double arr3[] = {1,2,3,4};
    polynom p1(2, arr1);
    polynom p2(1, arr2);
    polynom p3(3, arr3);
    cout << p1 << "\n";
    p1 = p2 = p3;
    cout << p1 << "\n";
    std::system("pause");
}

I get the following exception:

Unhandled exception at 0x01331C16 in 7.exe: 0xC0000005: Access violation reading location 0x01337000.

in this line this->arr[i] = p.arr[i];

I'm pretty clueless as to why.

Answer 1

There are many things wrong with your code. One of the is that you haven't defined a copy constructor but you are returning an object (*this) by value from your assignment operator. I would guess that the "copied" object points to something funny and the access to this pointer results in undefined behavior.

That said, I'm puzzled how anybody can cope with the complexity of the assignment operator! Your assignment operator has more problem:

1. It leaks memory if the object is actually pointing a actually allocated memory.
2. It wouldn't do too well with self-assignment.

Personally, I'm having trouble implementing assignment operators unless they leverage the copy constructor, destructor, and swap() function (of course, I'm only programming with C++ since nearly 25 years so maybe I'll figure it out at some point). The swap() may not exist, yet, but should implemented anyway:

polynom& polynom::operator= (polynom other) {
    this->swap(other);
    return *this;
}
void polynom::swap(polynom& other) {
    std::swap(this->grad, other.grad);
    std::swap(this->arr, other.arr);
}

Of course, this assumes you have a working copy constructor.

Here are other random problem:

1. As mentioned, the copy constructor needs a proper implementation.
2. In your assignment operator you don't verify if the right hand side polynom has a non-NULL pointer in arr but your default constructor initializes the arr member to NULL.
3. The using namespace std; should be after including a standard header, not before. Of course, headers should have a using-directive at all! At least in headers your are much better off using full qualification, e.g., std::ostream.
4. This one isn't quite a problem but a bit nitpicking: you shouldn't include <iostream> in headers unless you use std::cout, std::cin, std::cerr, or std::clog! I would include <iosfwd> in the header and <ostream> in the implementation and only <iostream> where things are actually written.

(I had claimed that there was another problem with just storing pointers to arrays but that's not the code; I removed it from the list; sorry).