C++:copy constructor

Question

I have a problem with my program in c++. I try to write copy constructor. Everything is good until I start copying elements from array from object that is applied as a reference. I can print every element of applied array but I can't copy it to array in object that copy constructor will create.

Here's the code (I show only the header file and some methods that needs copy constuctor because all code is too long I think)

Header.h

#pragma once
# include <iostream> 
# include <fstream> 
# include <cmath>
using namespace std;

class Polynomial {
private: 
    int degree; 
    double* coefficients; 


public: 
    static const string errors[5];
    Polynomial(const Polynomial& wzor); // konstruktor kopiujacy
    Polynomial(float n);
    ~Polynomial();
    void setCoefficient(unsigned int i, double value); 
    double getCoefficient(unsigned int i); 
    double value(double x); 
    friend ostream & operator <<(ostream & out, Polynomial & p);
    double operator[](const int index);
    double operator()(double x);
    void operator=(Polynomial &obiekt);
    friend Polynomial operator+(Polynomial &obiekt,Polynomial &obiekt1);
    friend Polynomial operator-(Polynomial &obiekt,Polynomial &obiekt1);
    friend Polynomial operator~(Polynomial &obiekt);
    friend Polynomial operator*(Polynomial &obiekt,Polynomial &obiekt1);
    friend Polynomial operator*(Polynomial &obiekt,double x);
};

Polynomial::Polynomial(float n) {

    if(n<0){
        throw 0;
    }
    else if(floor(n)-n != 0){
        throw 2;
    }

    degree=(int)n;
    coefficients=new double [(int)n+1]; 
    for(unsigned int i=0; i<n; i++) { 
        coefficients[i]=0.0; 
    } 
    coefficients[(int)n]=1.0; 
}

Polynomial::Polynomial(const Polynomial &wzor){
    degree=wzor.degree; // it's allright 
    for(int i=0; i<=wzor.degree; i++){
        coefficients[i]=wzor.coefficients[i]; // compilator says that this line is wrong 
    }
}

Polynomial::~Polynomial(){

    delete coefficients;

}

Polynomial operator+(Polynomial &obiekt,Polynomial &obiekt1){
    Polynomial nowy(1);
    if(obiekt1.degree > obiekt.degree){
        nowy=obiekt1;
        for(int i=0; i<=obiekt.degree; i++){
            nowy.coefficients[i]=nowy.coefficients[i]+obiekt.coefficients[i];
        }
    }
    else{
        nowy=obiekt;
        for(int i=0; i<=obiekt1.degree; i++){
            nowy.coefficients[i]=nowy.coefficients[i]+obiekt1.coefficients[i];
        }
    }
    return nowy;
}

Warning sign: You have a `double *` in your class, but nothing allocates new storage behind that `double *` in your copy constructor. Why aren't you using `vector` here anyway? — Joe Z, Dec 03 '13 at 14:01
warning sign2: you are using `new[]` for `coefficients`, but are not matching it with `delete[]` — Hulk, Dec 03 '13 at 14:03

Felix Glas · Accepted Answer · 2013-12-03T14:34:07.387

class Polynomial {
private: 
    int degree; 
    double* coefficients; // Pointer to double

    /* ... */
}

You have declared coefficients to be a pointer to double, not an array.

for(int i = 0; i <= wzor.degree; i++) {
    coefficients[i] = wzor.coefficients[i]; // Error! No memory allocated.
}

Here you are trying to assign values to memory that doesn't exist. You need to allocate the array before you can assign element data to it. Also you should initialize members in the initialization list instead of the ctor body. Try something like this instead:

Polynomial::Polynomial(const Polynomial& wzor)
    : degree(wzor.degree), coefficients(new double[wzor.degree + 1]) { // Init
    for (int i=0;i<=wzor.degree;i++) {
        coefficients[i]=wzor.coefficients[i];
    }
}

Remember that the copy constructor is a constructor. It will construct a new instance of the object and you need to allocate new memory for the new copy.

Also in your destructor you must use delete[] instead of delete as you are deleting an array.

The assignment operator should be declared as:

Polynomial& operator= (const Polynomial& obiekt);

You should use type std::size_t for variables storing array indexes instead of int.

Finally I recommend using containers from the standard library instead of built in arrays, e.g. std::vector.

You're right this was a problem. One else question. Why the assignment operator should be declared as: Polynomial& operator= (const Polynomial& obiekt); I want to assign something so I want to change an object that I am working with, I don't want to get new object that is copy of object applied as argument. — user3061714, Dec 04 '13 at 01:06
@user3061714 The definition of `Polynomial& operator= (const Polynomial& obiekt)` should return a *reference* to the object assigned to (e.g. `{degree=obiekt.degree;return *this;}`), not a copy of the argument (it's declared as returning a reference). This is the usual way to declare copy assignment operator as it allows expressions like `if ((x = y)) {}` and `x = y = z` (operator chaining). See this for more info: http://stackoverflow.com/questions/9072169/why-should-the-assignment-operator-return-a-reference-to-the-object — Felix Glas, Dec 04 '13 at 10:45

score 2 · Answer 2 · answered Dec 03 '13 at 14:12

#include <vector>

std::vector<double> coefficients;

you can use the copy of the vector which means you do not have to implement it on your own. And you do not have to do the delete stuff.

Use resize & [] or push_back methods to work on the string. Your degree will be coefficients.size().

score 0 · Answer 3 · answered Dec 03 '13 at 14:10

0

You're assignment operator is wrong. It should be

Polynomial & operator=(const Polynomial &obiekt);

answered Dec 03 '13 at 14:10

C++:copy constructor

3 Answers3