c++, Object of class changes when creating a new object

Question

I have created a class matrix that besides having some member functions, operators and a constructor has two variables:

int m, which is the dimension (side length) of the quadratic matrix

double a[], which contains the elements of the matrix.

As long as I only create one matrix object A all seem to work fine and all my member operators and functions work as they should.

My problem is that once I create a second object B of the class matrix the variables of object A changes as well.

The relevant code is as follows:

class Matrix{
    private:
        const int m;
        double a[];
    public:
        Matrix(int inpm);
        void fillMatrix(const double inpa[]);
};
Matrix::Matrix(int inpm): m(inpm){
    a[impm*inpm];
}
void Matrix::fillmatrix(const double inpa[]){
    for (int i ; i<m*m ; i++){
        a[i]=inpa[i];
    }
}

int min =2;
double ain[min*min] = {1,2,3,4};
double bin[min*min] = {5,6,7,8};

Matrix A(min);
Matrix B(min);

A.fillMatrix(ain);
//A looks precisely as it should here:
//m=2, a={1,2,3,4}
B.fillMatrix(bin);
//Here B looks as it should but A has changed to:
//m=0, a={7,8,3,4}

obviously the change to the first object occur when I run fillMatrix() for the second object but I cannot figure out why, especially since m is a constant integer.

I'm really stuck so grateful for all help.

PS: I use another member function "void printMatrix();" using std:cout to look at the values of m and all elements in a.

Answer 1

a[impm*inpm]; This does not do what you think. GCC even warns with statement has no effect.

a remains uninitialized here and you run into undefined behavior when you try to access a[i]

class Matrix{
private:
    const int m;
    double *a;
    ....

and

Matrix::Matrix(int inpm): m(inpm)
{
   a = new double[inpm*inpm];
}

and then remember to delete

Matrix::~Matrix()
{
   delete a;
}

Answer 2

c++ arrays are not actually allowed to variable. There are plenty of sources about why variable length arrays (vla's) are not allowed. If you wan't a dynamic length array with size chosen at run time, you need to allocate some memory:

double *a;
...
a = new double[inpm*inpm];

But this sucks! Now you have to remember to delete and access correctly and everything. You could wrap this memory in a class to control this, and since this is such a good idea, c++ provides this as standard. It is called std::vector. Your code will happily reduce to this:

class Matrix{
private:
    const int m;
    std::vector<double> a;
public:
    Matrix(int inpm) : m(inpm), a(m * m) {}

Answer 3

I've noticed a few issues with your code. The first few were already mentioned by others such as your member double a[]; This is uninitialized and C++ does not allow for variable length vectors. To resolve this there are two possible solutions which I'll discuss shortly. The other issue that others have mentioned is that in your Matrix::fillMatrix() function the variable in your for loop i is also uninitialized, so i can be anything and this will lead to undefined behavior.

A few issues that have not been mentioned by others are as follows:

In your constructor definition you are trying to initialize m with a[impm*inpm] I think this might be a typo on your part. Another one is you declare your function as ::fillMatrix() but you define it outside of the class declaration as ::fillmatrix(). Again I think this might be just a typo on your part.

As for the issue above with the use of arrays in C++ the easiest way to do this is what others have already stated and that is to use std::vector<type>.

---

The other way is to write a class that works similarly to std::vector but has the mechanics of a matrix. Your class would look something like this using templates: If you want a variable length container at runtime you have to know it's size at compile time using templates here can help as the template has to be deduced at compile time upon instantiation!

template<class T, unsigned N>
class Matrix {
private:
    static const unsigned Stride = N;
    static const unsigned Size = Stride * Stride;
    T data[Size] = {};

public:
    Matrix() {};

    void fillMatrix( const T* dataIn );
    void printMatrix();
};

template<class T, unsigned N>
void Matrix<T, N>::fillMatrix( const T* dataIn ) {
    for( unsigned int i = 0; i < Size; i++ ) {
        this->data[i] = dataIn[i];
    }
}

template<class T, unsigned N>
void Matrix<T, N>::printMatrix() {
    for( unsigned int i = 0; i < Stride; i++ ) {
        for( unsigned int j = 0; j < Stride; j++ ) {
            std::cout << this->data[i*Stride + j] << " ";
        }
        std::cout << '\n';
    }
}

int main() {        
    // 2x2 = 4
    double ain[4] = { 1,2,3,4 };
    double bin[4] = { 5,6,7,8 };

    Matrix<double, 2> A;
    Matrix<double, 2> B;

    A.fillMatrix( ain );
    A.printMatrix();
    std::cout << '\n';

    B.fillMatrix( bin );
    B.printMatrix();
    std::cout << '\n';

    // test A again
    A.printMatrix();
    std::cout << '\n';
    B.printMatrix();
    std::cout << '\n';    

    // Try it with other types
    // 3x3 = 9
    char data[9] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i' };
    Matrix<char, 3> C;
    C.fillMatrix( data );
    C.printMatrix();
    std::cout << '\n';

    return 0;
}

Output:

1 2 
3 4

5 6
7 8

1 2 
3 4

5 6
7 8

a b c
d e f 
g h i

With this type of design using templates you are not restricted to just using a single type such as int, float etc. You can even use User Defined Types. The only thing here that you would need to consider is any kind of operators you may have. Also the way this class is written it will always create a MxM matrix which will always be a square matrix. If you need a non square matrix; you can just add to this class by adding in a 2^nd unsigned constant value to the template parameter list and renaming them accordingly for MxN. Then just substitute where they belong in the math. It would look something like this:

template<class T, unsigned M, unsigned N>
class Matrix {
private:
    static const unsigned Row = M;
    static const unsigned Col = N;
    static const unsigned Size = Row * Col;

    T data[Size] = {};
};

Now how you label them might depend on if you want Row-Col major or Col-Row major...

Answer 4

The problem is because of your array declaration. people used to call that kind of array declaration as "flexible array declaration.". And it wouldn't work as you expected. plus there are certain rules to follow while using a flexible array. But if you want to create a dynamic array you can use malloc to create array dynamically.