Why is this object is not getting copied by value

Question

When I return the object temp in the operator* function, it is not getting copied to p3 in the main.

The cout statements inside the operator* function is returning correct values but p3 in main has only garbage.

Also I am getting a _block_type_is_valid(phead->nblockuse) at the return 0 statement of main.

Here is the code.

#pragma once
#include<iostream>
using namespace std;
class Polynomial
{
private:
    int *coefficients;
    int length;
public:
    inline int getLength() const {return length;}
    Polynomial(int length);
    Polynomial(const Polynomial &p);
    Polynomial():coefficients(nullptr){}
    ~Polynomial(void);
    extern friend Polynomial operator*(const Polynomial &p1,const Polynomial &p2);
    inline int operator[](int n) const { return coefficients[n];}
    inline int& operator[](int n) { return coefficients[n];}
    Polynomial& operator=(Polynomial &p);
    friend void swap(Polynomial& first, Polynomial& second);
    void resize(int x);

    friend istream& operator>>(istream &is, Polynomial &p);
    friend ostream& operator<<(ostream &os, Polynomial &p);
};

Here is polynomial.cpp

#include "Polynomial.h"

Polynomial::Polynomial(int length){
    this->length = length;
    coefficients = new int[length];
    for(int i = 0; i < length; i++){
        coefficients[i] = 0;
    }
}


Polynomial::~Polynomial(void){
    cout<<"Deleting: "<<coefficients;
    delete[] coefficients;
}
/*
Polynomial Polynomial::operator*(Polynomial p){
Polynomial temp(length + p.getLength());

for(int i = 0; i < length; i++){
for(int j = 0; j < length; j++){
temp[i+j] += coefficients[i] * p[j];            
}
}   
cout<<temp;
return temp;
}*/

Polynomial operator*(const Polynomial &p1,const Polynomial &p2){
    Polynomial temp(p1.getLength() + p2.getLength());

    for(int i = 0; i < p1.getLength(); i++){
        for(int j = 0; j < p2.getLength(); j++){
            temp[i+j] += p1[i] * p2[j];         
        }
    }   
    cout<<temp;
    return temp;
}
void Polynomial::resize(int x){
    delete[] coefficients;
    coefficients = new int[x];
}
void swap(Polynomial& first,Polynomial& second){
    int tempLength = first.getLength();
    int *temp = new int[tempLength];
    for(int i = 0; i < first.getLength(); i++)
        temp[i] = first[i];
    first.resize(second.getLength());
    for(int i = 0; i < first.getLength(); i++)
        first[i] = second[i];
    second.resize(tempLength);
    for(int i = 0; i < first.getLength(); i++)
        second[i] = temp[i];
    delete[]temp;
}
Polynomial& Polynomial::operator=(Polynomial &p){
    swap(*this,p);
    return *this;
}
Polynomial::Polynomial(const Polynomial &p){
    //if(coefficients) delete [] coefficients;
    coefficients = new int[p.getLength()];
    for(int i = 0; i < p.getLength(); i++)
        coefficients[i] = p[i];
}
istream& operator>>(istream &is,Polynomial &p){
    cout<<"Enter length: ";
    is>>p.length;
    p.coefficients = new int[p.length];
    for(int i = 0; i < p.length; i ++)
        is>>p.coefficients[i];  
    return is;
}
ostream& operator<<(ostream &os,Polynomial &p){ 
    for(int i = 0; i < p.length; i ++)
        if(p.coefficients[i])
            os<<p.coefficients[i]<<"x^"<<i<<" ";        
    return os;
}

Here is main

#include"Polynomial.h"

#include<iostream>
#include<string>

using namespace std;

int main(){
    Polynomial p1,p2,p3;
    cin>>p1>>p2;
    p3 = (p1 * p2);
    cout<<p3[0]<<p3[1]<<"here";
    cout<<p3;

    return 0;
}

EDIT: Here is the final corrected code. All I needed to do is initialize the pointers with null and length with 0 in all constructors.

#include "Polynomial.h"

Polynomial::Polynomial():
    coefficients(nullptr),length(0){}

Polynomial::Polynomial(int length):coefficients(nullptr),length(length){    
    coefficients = new int[length];
    for(int i = 0; i < length; i++){
        coefficients[i] = 0;
    }
}

Polynomial::~Polynomial(void){  
    if(coefficients) delete[]coefficients;
}

Polynomial& Polynomial::operator=(const Polynomial &p){ 
    if(coefficients)    
        delete[]coefficients;

    length = p.getLength();
    coefficients = new int[length]; 
    for(int i = 0; i < length; i++)
        coefficients[i] = p[i]; 

    return *this;
}
Polynomial::Polynomial(const Polynomial &p):
    coefficients(nullptr),length(0)
{   
    length = p.getLength();
    coefficients = new int[length]; 
    for(int i = 0; i < length; i++)
        coefficients[i] = p[i];     
}

Polynomial operator*(const Polynomial &p1,const Polynomial &p2){
    Polynomial temp(p1.getLength() + p2.getLength());
    for(int i = 0; i < p1.getLength(); i++)
        for(int j = 0; j < p2.getLength(); j++)
            temp[i+j] += p1[i] * p2[j];         
    return temp;
}

istream& operator>>(istream &is,Polynomial &p){
    cout<<"Enter length: ";
    is>>p.length;
    p.coefficients = new int[p.length];
    for(int i = 0; i < p.length; i ++)
        is>>p.coefficients[i];  
    return is;
}
ostream& operator<<(ostream &os,Polynomial &p){ 
    for(int i = 0; i < p.length; i ++)
        if(p.coefficients[i])
            os<<p.coefficients[i]<<"x^"<<i<<" ";        
    return os;
}

Answer 1

This code must not compile – if you invoke your compiler with the right flags. There is no C++ compiler which works well with default flags!

For example, let's take Visual C++ 2013, invoked like this, with a few non-default flags:

cl /nologo /EHsc /Za /W4 stackoverflow.cpp

The result is, correctly, a compiler error:

stackoverflow.cpp(78) : error C2679: binary '=' : no operator found which takes a right-hand operand of type 'Polynomial' (or there is no acceptable conversion)

        stackoverflow.cpp(19): could be 'void Polynomial::operator =(Polynomial &)'
        while trying to match the argument list '(Polynomial, Polynomial)'

Now let's see what happens if we remove the /Za flag, which disables Microsoft extensions:

cl /nologo /EHsc /W4 stackoverflow.cpp

The error goes away. However, a warning appears:

warning C4239: nonstandard extension used : 'argument' : conversion from 'Polynomial' to 'Polynomial &'
        A non-const reference may only be bound to an lvalue; assigment operator takes a reference to non-const

And finally, let's see what the compiler does if we remove /W4, the warning-level flag:

cl /nologo /EHsc stackoverflow.cpp

No errors, no warnings. That's bad. p3 = (p1 * p2); is not standard C++. Even though you can make Visual C++ compile the code, nothing good will come of it.

The right signature of the assignment operator would be:

Polynomial& Polynomial::operator=(Polynomial const& p);

The biggest problem with your code, however, is the use of new[] and delete[]. You certainly have a memory leak somewhere (construction and assignment both allocate, but only the destructor deletes), and even worse, you delete the same array multiple times.

Add the following output to your destructor:

Polynomial::~Polynomial(void){
    std::cout << "coefficients address destructor: " << coefficients << "\n";
    delete[] coefficients;
}

If you run it now, you will see something like:

Enter length: 2
100
200
Enter length: 3
100
200
300
10000x^0 40000x^1 40000x^2 coefficients address destructor: 00463BF8
coefficients address destructor: 00463BF8
1040000herecoefficients address destructor: 00463BF8

Three times 00463BF8! This is undefined behaviour and can lead to every imaginable thing to happen in your program.

But how can this be?

The answer is that your operator* returns a shallow copy of a Polynomial object. The pointer inside is copied, and you end up with two pointers pointing to the same allocated memory.

You need a copy constructor. And you must respect the so-called Rule Of Three: if you need to implement one of copy constructor, copy assignment operator and destructor, then you need to implement all of them.

Nevertheless, instead of taking all the trouble and implement dynamic allocation with its countless pitfalls manually, do yourself a favour and use std::vector instead of a pointer and a length variable:

class Polynomial
{
private:
    std::vector<int> coefficients;
    // ...
};

Your subscript operator also needs modification. In fact, you'll need two of them, one for const access and one for non-const access:

inline int operator[] (int n) const { return coefficients[n];}
inline int& operator[] (int n) { return coefficients[n];}

See why do subscript operators C++ often comes in pair?

Finally, I think you misunderstand extern. You don't need it to use friend functions. Just remove it from your code; otherwise GCC won't even compile it.

Here is a complete example with the most important fixes:

#include <iostream>
#include <vector>

using namespace std;

class Polynomial
{
private:
    std::vector<int> coefficients;
public:
    inline int getLength() const {return coefficients.size();}
    Polynomial(int length);
    Polynomial(){}
    Polynomial operator*(Polynomial &p);
    inline int operator[] (int n) const { return coefficients[n];}
    inline int& operator[] (int n) { return coefficients[n];}

    friend istream& operator>>(istream &is, Polynomial &p);
    friend ostream& operator<<(ostream &os, Polynomial &p);
};

Polynomial::Polynomial(int length) :
    coefficients(length)
{
    for(int i = 0; i < length; i++){
        coefficients[i] = 0;
    }
}

Polynomial Polynomial::operator*(Polynomial &p){
    Polynomial temp(coefficients.size() + p.getLength());

    for(int i = 0; i < coefficients.size(); i++){
        for(int j = 0; j < coefficients.size(); j++){
            temp[i+j] += coefficients[i] * p[j];            
        }
    }   
    cout<<temp;
    return temp;
}

istream& operator>>(istream &is,Polynomial &p){
    cout<<"Enter length: ";
    int length;
    is>>length;
    p.coefficients.clear();
    p.coefficients.resize(length);
    for(int i = 0; i < length; i ++)
        is>>p.coefficients[i];  
    return is;
}
ostream& operator<<(ostream &os,Polynomial &p){ 
    for(int i = 0; i < p.coefficients.size(); i ++)
        if(p.coefficients[i])
            os<<p.coefficients[i]<<"x^"<<i<<" ";        
    return os;
}

int main(){
    Polynomial p1,p2,p3;
    cin>>p1>>p2;
    p3 = (p1 * p2);

    cout<<p3[0]<<p3[1]<<"here";
    cout<<p3;

    return 0;
}

You don't need a self-written copy constructor, copy assignment operator or destructor anymore, because std::vector knows how to copy, assign and destruct itself, in a safe manner without low-level memory problems.

Further examples for future improvement:

• Check that std::cin actually received valid integers.
• Implement operator* in terms of operator*=.
• Make operator* take a const reference.
• Don't use using namespace at global scope in header files.

Generally, read up on const and on best practices for operator overloading.

Answer 2

Assuming it compiles.

You do not have a copy constructor, it is not a good idea. If you implement a destructor, you probably need assign-copy-swap-move functions. What is The Rule of Three? (and a half)

Also, it is better to use std::vector instead of dynamic allocation, since it automatically implements these behaviours, and the compiler generated constructors are enough then.

In your case returning creates a shallow copy (auto generated copy-cstr), but after the function returns, the original gets destroyed - which has allocated the memory the shallow copy is referencing - and freeing the memory.

It could be enough:

class Polynomial
{
private:
    std::vector<int> coefficients;

public:
    inline int getLength() const { return coefficients.size(); };
    Polynomial(int length): coefficients(length, 0) {};
    Polynomial operator*(const Polynomial& p); //you are not modifying p, thus const ref can work

    extern friend istream& operator>>(istream &is, Polynomial &p);
    extern friend ostream& operator<<(ostream &os, Polynomial &p);
};

Alternatively, you can use the array solution, and implement it right with all of the necessary stuff:

class Polynomial
    {
    public:
        Polynomial(int length);
        ~Polynomial();
        Polynomial(const Polynomial& other);
        friend void swap(Polynomial& first, Polynomial& second) // nothrow
        Polynomial& operator=(Polynomial other)
        {
            swap(*this, other);
            return *this;
        }

        int getLength();
        Polynomial operator*(const Polynomial& p); //you are not modifying p, thus const ref can work

        extern friend istream& operator>>(istream &is, Polynomial &p);
        extern friend ostream& operator<<(ostream &os, const Polynomial &p);
    };

This is the copy swap idiom.

Edit: you need a inline int operator[](int n) const { return coefficients[n];} too.