1

I am trying to create a way to resize my dynamically allocated array. I am able to add rows and columns, however resizing it to be smaller than the original size is not working.

template <class T>
class Matrix
{
public:

// Constructors / Destructors
Matrix();
Matrix(const int rows, const int cols);
Matrix(const Matrix<T>& orig);
~Matrix();

// Display
void print() const;

// Size modification
void addRow();
void addCol();
void resize(const int rows, const int cols);

// Element access
T getCell(const int r, const int c) const;

// Operator Access
T& operator()(const int r, const int c)             { return mData[r * mCols + c]; }
const T& operator()(const int r, const int c) const { return mData[r * mCols + c]; }



// Getters / Setters
int getRows() const;
int getCols() const;

private:
    T* mData;
    int mRows, mCols;
};
//1
template <class T>
Matrix<T>::Matrix(void){
    mRows=0;
    mCols=0;
    mData=NULL;
}

//2
template <class T>
Matrix<T>::Matrix(const int rows, const int cols){
    if(rows!=0&&cols!=0)
    {
        mRows=rows;
        mCols=cols;
        int size = rows*cols;
        mData = new T[size];
        for(int r=0;r<getRows();r++)
        {
            for(int c=0;c<getCols();c++)
            {
                mData[r * mCols + c]=0;
            }
        }
    }else
    {
        mData=NULL;
    }
}

//3
template <class T>
Matrix<T>::Matrix(const Matrix<T>& orig){
    if(orig.getRows()!=0&&orig.getCols()!=0)
    {
        T* temp = mData;
        mRows=orig.getRows();
        mCols=orig.getCols();
        int size = mRows*mCols;
        mData = new T[size];
        for(int r=0;r<getRows();r++)
        {
            for(int c=0;c<getCols();c++)
            {
                mData[r * mCols + c]=temp[r * mCols + c];
            }
        }
    }else
    {
        mData=NULL;
    }
}

//4
template <class T>
Matrix<T>::~Matrix()
{
    delete []mData;
    mData=NULL;
}

//5
template <class T>
void Matrix<T>::print() const{  //to print the mData in a Matrix form
    cout << endl;
    int r=0,c=0,w=10;
    for(r=0;r<getRows();r++){
        for(c=0;c<getCols();c++){
            cout << setw(w) << mData[r * mCols + c];
        }
        cout << endl;
    }
}

//6
template <class T>
void Matrix<T>::addRow(){
    T* temp = mData;
    mRows=getRows()+1;
    int size = mRows*mCols;
    mData = new T[size];
    for(int r=0;r<getRows();r++)
    {
        for(int c=0;c<getCols();c++)
        {
            if(r==getRows()-1)
            {
                mData[r * mCols + c]=0;
            }else
            {
                mData[r * mCols + c]=temp[r * mCols + c];
            }

        }
    }
}

//7
template <class T>
void Matrix<T>::addCol(){
    T* temp = mData;
    mCols=getCols()+1;
    int size = mRows*mCols;
    mData = new T[size];
    for(int r=0;r<getRows();r++)
    {
        for(int c=0;c<getCols()-1;c++)
        {
                mData[r * mCols + c]=temp[r * (mCols-1) + c];
        }
    }
    int c=getCols()-1;
    for(int r=0;r<getRows();r++)
    {
            mData[r * mCols + c]=0;
    }
}

//8
template <class T>
void Matrix<T>::resize(const int rows, const int cols){


    if(rows>=getRows())
    {
        while(rows!=getRows())
        {
            addRow();
        }
    }

    if(cols>=getCols())
    {
        while(cols!=getCols())
        {
            addCol();
        }
    }


    T* temp = mData;
    int tempCols = getCols();
    int size = rows*cols;
    if(rows<getRows() || cols <getCols())
    {
        delete []mData;
        mData=NULL;
        mData= new T[size];
        for(int r=0;r<getRows();r++)
        {
            for(int c=0;c<getCols();c++)
            {
                    if(r<rows&&c<cols)
                    {
                        mData[r * mCols + c]=temp[r * (cols) + c];
                    }else
                    {
                        mData[r * mCols + c]=NULL;
                    }

            }
        }
    }

}


//9
template <class T>
T Matrix<T>:: getCell(const int r, const int c) const
{
    T value;
    if(r<getRows()&&c<getCols()&&r>=0&&c>=0)
    {
        value = mData[r * mCols + c] ;
    }else
    {
        value = 0;
    }
    return(value);
}

//22
template <class T>
int Matrix<T>::getRows() const{
    return mRows;
}
//23
template <class T>
int Matrix<T>::getCols() const{
    return mCols;
}
#endif

All the other functions that I have implemented works perfectly and i added the implantation to help with testing.

Currently if you have a matrix that is 3,4 and you want to increase to 4,7 it will successfully do so. However if you try and truncate it to 2,2 it will still stay 3,4 even though according to the new size and for loop it should have been smaller.

so My question is that if I have a (3,4) Matrix M1

1 2 3 4
6 7 8 2
3 2 1 1

if I run m1.resize(2,7) I want to get

1 2 3 4 0 0 0
6 7 8 2 0 0 0

However currently I get

3801280 3802496 2 3 4 0 0 
6       7       8 2 0 0 0
0       0       0 0 0 0 0
0       0       0 0 0 0 0

What's wrong with my implementation of resize?

  • What is your question statement? "Something explodes" is not a question – Passer By Jul 10 '17 at 03:53
  • 1
    Why not just hold `std::vector` internally? – Charles Jul 10 '17 at 03:54
  • Unfortunatelly I can't because the assignment is to understand Abstract Data Types (ADTs) by implementing a Matrix class using a dynamically allocated array. – Moustafa ELhadary Jul 10 '17 at 03:56
  • @PasserBy I just edited the question, I hope it i have made my question clearer now. – Moustafa ELhadary Jul 10 '17 at 04:06
  • `T* temp = mData;` does not perform deep copy. When you afterwards `delete []mData;` the data `temp` is pointing to is also gone. Dereferencing `temp` afterwards triggers undefined behaviour. This is very unfortunate, because that means you might sometimes get the correct result at random and most of the time it blows up. A segfault would have been nicer. – Henri Menke Jul 10 '17 at 04:25
  • @HenriMenke Thank you so much!! That was great help, I went back and performed a Deep Copy and also made sure `mRows=rows;mCols=cols` and now it is working like I intended! Thank you!! – Moustafa ELhadary Jul 10 '17 at 05:19

1 Answers1

1

T* temp = mData; does not perform deep copy. When you afterwards delete []mData; the data temp is pointing to is also gone. Dereferencing temp afterwards triggers undefined behaviour. This is very unfortunate, because that means you might sometimes get the correct result at random and most of the time it blows up. A segfault would have been nicer.

Here is a short example. I also put the line double * temp = mData; which does not perform deep copy.

#include <algorithm>
#include <iostream>

int main()
{
  double * mData = new double[5];
  std::fill_n(mData, 5, 1);

  double * temp = mData; // Wrong! No deep copy
  //double * temp = new double[5];
  //std::copy(mData, mData+5, temp);

  delete[] mData;
  mData = new double[10];
  std::fill_n(mData, 10, 0); // fill with zeros
  for (int i = 0; i < 5; ++i)
    mData[i] = temp[i];

  for(int i = 0; i < 10; ++i)
    std::cout << mData[i] << ' ';
  std::cout << '\n';

  delete[] temp;
  delete[] mData;
}

The output of this program might or might not be fine. After all the behaviour is undefined. It is best to run programs with manual memory management in a memory debugger (e.g. valgrind on Linux) to detect the invalid operations. If I run the above in valgrind I get (among some generic messages)

==9998== Invalid read of size 8
==9998==    at 0x400908: main (in /home/henri/a.out)
==9998==  Address 0x5abfc80 is 0 bytes inside a block of size 40 free'd
==9998==    at 0x4C2F74B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9998==    by 0x4008C1: main (in /home/henri/a.out)
==9998==  Block was alloc'd at
==9998==    at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9998==    by 0x40087A: main (in /home/henri/a.out)
==9998== 
1 1 1 1 1 0 0 0 0 0 
==9998== Invalid free() / delete / delete[] / realloc()
==9998==    at 0x4C2F74B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9998==    by 0x4009A9: main (in /home/henri/a.out)
==9998==  Address 0x5abfc80 is 0 bytes inside a block of size 40 free'd
==9998==    at 0x4C2F74B: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9998==    by 0x4008C1: main (in /home/henri/a.out)
==9998==  Block was alloc'd at
==9998==    at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9998==    by 0x40087A: main (in /home/henri/a.out)
Henri Menke
  • 10,705
  • 1
  • 24
  • 42