0

I am writing program which will multiply matrix using threads. I have problem with filling dynamic int array (cannot use vector).

cpp file:

Matrix::Matrix(int n, int m)
{
    mac_ = new int[n * m];
}

Matrix::Matrix(std::istream & is)
{
    int tmp;
    int i = 0;  
    is >> m_; //rows
    is >> n_; //columns

    Matrix(n_, m_); // using 1st constructor

    while (is.peek() != EOF) {
        is >> tmp;
        mac_[i] = tmp; // debug stop here
        i++;
    }
}

part of hpp file:

private:
    int n_; // columns
    int m_; // rows
    int *mac_;

From debug I get:

this 0x0079f7b0 {n_=3 m_=2 mac_=0x00000000 {???} }

I know that i can write mac_ = new int(n_*m_); in 2nd constructor but I want to know why 1st does not work.

rocambille
  • 15,398
  • 12
  • 50
  • 68
Xalion
  • 623
  • 9
  • 27
  • "Using 1st.." Creates a temporary object, not an allocation for the intended object – Jonas Nov 26 '16 at 19:35
  • You could use delegated constructors – Jonas Nov 26 '16 at 19:36
  • @Jonas he can't use delegated constructor, since he first has to use the `istream` given as argument to obtain construction parameters. Good old `init` function is here better suited – rocambille Nov 26 '16 at 19:38
  • @wasthishelpful I stand corrected. – Jonas Nov 26 '16 at 19:40
  • 2
    Before you go much further, make sure `Matrix` adheres to the Rule of Three or you'll be back here with a new question before long. [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – user4581301 Nov 26 '16 at 20:38

1 Answers1

2
// ...

Matrix(n_, m_); // using 1st constructor

while (is.peek() != EOF) {
    is >> tmp;
    mac_[i] = tmp; // debug stop here
    i++;
}

Looks like you think calling the constructor here constructs the actual object (this), and then you access its member attribute mac_.

In fact, calling the constructor as you did creates an other object unrelated to this matrix (since you're not storing it in variable, it is destructed at the end of the line).

So because you constructed an other object instead of this, this->mac_ is uninitialized, thus your bug.

Modify your code like this:

Matrix::Matrix(int n, int m)
{
    init(n, m);
}

Matrix::Matrix(std::istream & is)
{
    int tmp;
    int i = 0;  
    is >> m_; //rows
    is >> n_; //columns

    init(n_, m_);

    while (is.peek() != EOF) {
        is >> tmp;
        mac_[i] = tmp; // debug should not stop here anymore
        i++;
    }
}

void Matrix::init(int n, int m)
{
    mac_ = new int[n * m];
}

Note: here I take out the inititialization in a function init (should probably be a private method) to avoid code duplication.

rocambille
  • 15,398
  • 12
  • 50
  • 68