dynamic array member variables

Question

Hey i'm new to c++ and still working out its perticularities. I'm having the darnedest time trying to figure out whats going wrong with this code. I've stepped through it and everything is calculating correctly. The issue is that value_array in the base class doesn't seem to be retaining the values once the derived class Calculate function ends. I think i've declared and allocated the array properly. I'm stumped...

#include <iostream>

class Indicator 
{
protected:
    double * value_array;
    double * input_array;
    int input_size;
public:
    Indicator(double input[], int size) 
    { 
        input_array = input; 
        input_size = size; 
        value_array = new double[size]; // issue with value_array
    }
    double operator[] (int index) { return value_array[index]; }
    void virtual Calculate() {}
    ~Indicator() { delete[] value_array; }
};

class SMA : public Indicator 
{
private:
    int nperiod;
    double sum;
public:
    SMA(double input[], int size, int period) : Indicator(input, size) 
    { 
        nperiod = period; 
        sum = 0; 
        Calculate(); 
    }
    void Calculate();
};

void SMA::Calculate()
{ 
    for (int i=0; i<input_size; i++)
    {
        if (i > nperiod - 1) 
        {
            sum += input_array[i] - input_array[i-nperiod];
            value_array[i] = sum / nperiod;
        }
        else
        {
            sum += input_array[i];
            value_array[i] = sum / (i+1);
        }
    }
}

int main(int argc, const char *argv[]) {
    double input[] = {1,2,3,4,5,6,7,8,9,10};
    Indicator indicator = SMA(input,10,5);

    double value = indicator[0];
    std::cout << "value: " << value << std::endl;

    std::cin.get();
    exit(0);
}

Update:

Here is the code implemented with vectors. I wanted to leave the input as double[] to be consistent with other libraries, any other potential issues I should be aware of?

#include <iostream>
#include <vector>

class Indicator 
{
protected:
    std::vector<double> value_vector;
    double * input_array;
    int input_size;
public:
    Indicator(double input[], int size) 
    { 
        input_array = input; 
        input_size = size; 
        value_vector.reserve(size);
    }
    double operator[] (int index) { return value_vector[index]; }
    void virtual Calculate() {}
};

class SMA : public Indicator 
{
private:
    int nperiod;
    double sum;
public:
    SMA(double input[], int size, int period) : Indicator(input, size) 
    { 
        nperiod = period; 
        sum = 0; 
        Calculate(); 
    }
    void Calculate();
};

void SMA::Calculate()
{ 
    for (int i=0; i<input_size; i++)
    {
        if (i > nperiod - 1) 
        {
            sum += input_array[i] - input_array[i-nperiod];
            value_vector.push_back(sum / nperiod);
        }
        else
        {
            sum += input_array[i];
            value_vector.push_back(sum / (i+1));
        }
        std::cout << "sma:  " << value_vector[i] << std::endl;
    }
}

int main(int argc, const char *argv[]) {
    double input[] = {1,2,3,4,5,6,7,8,9,10};
    Indicator indicator = SMA(input,10,5);

    for (int i=0; i<10; i++)
    {
        std::cout << "main: " << indicator[i] << std::endl;
    }

    std::cin.get();
    exit(0);
}

Although a useful exercise for education, please use a `std::vector` in real code. — , Feb 13 '11 at 15:51
@delnan care to explain why not? maybe a link explaining the problems and solutions — thecoshman, Feb 13 '11 at 15:53

score 3 · Accepted Answer · edited May 23 '17 at 10:24

3

That's because you're violating the Rule of Three. Since your class manages a resource, it needs a copy constructor and an assignment operator. I strongly suggest replacing any T* data member with a std::vector<T> data member. Then you don't need to write those special member functions manually.

edited May 23 '17 at 10:24

Community

1
1

answered Feb 13 '11 at 15:54

fredoverflow

256,549
94
388
662

score 2 · Answer 2 · edited May 23 '17 at 10:33

Hia,

a few things are wrong.

As FredOverflow says you need a copy constructor and assignment, something like:

Indicator::Indicator(const Indicator& other)
{
  input_size = other.input_size;

  //direct copy of reference as indicator doesn't own this data
  //Note a shared pointer (such as boost::shared_ptr) would be better than a naked reference
  input_array = other.input_array; 
  //construct a new set of data
  value_array = new double[input_size]; 
  //do you want to copy the data too? maybe a memcpy follows?
  memcpy(value_array, other.value_array, input_size*sizeof(double));
}

Then you need an assignment

Indicator&
Indicator::operator=(const Indicator& other)
{
  //make sure you are not assigning itself
  if(this != &other)
  {
    input_size = other.input_size;

    //direct copy of reference as indicator doesn't own this data
    //Note a shared pointer (such as boost::shared_ptr) would be better than a naked reference
    input_array = other.input_array; 
    //destroy old data and construct a new set of data
    delete[] value_array;
    value_array = new double[input_size]; 
    //do you want to copy the data too? maybe a memcpy follows?
    memcpy(value_array, other.value_array, input_size*sizeof(double));
  }
  return *this;
}

You probably also want to make the destructor virtual - see here for why - it helps prevent memory leaks in the destructor of SMA

virtual ~Indicator() { delete[] value_array; }

Thanks Tom, was wondering how it could be implmented with double *. Coming from a managed c background I never realized how much c#/java handles for you :P — darckeen, Feb 13 '11 at 16:54
@user615174 Not sure what you mean? This is with the double* as in your original question. The other answers tell you to use a vector because they handle the deletes & reasignments (done above) that you need because you are copying a pointer to an array. The default copy and assignment operators would work if you used std::vector. They don't work with a double because you want to manage memory. — Tom, Feb 13 '11 at 17:00

score 0 · Answer 3 · answered Feb 13 '11 at 16:34

0

Use std::vector instead of raw arrays.

std::vector handles all the memory management and copying and so forth.

Cheers & hth.,

answered Feb 13 '11 at 16:34

Cheers and hth. - Alf

142,714
15
209
331

dynamic array member variables

Update:

3 Answers3