Using built in move function gives segmentation fault while trying to make matrix using 2D vector

[llll]

I am trying to make matrix from 2D vector, whenever I comment out the lines that call built in move functions I get segmentation fault,if I comment them out everything runs smoothly and I get perfect answer except the ones that have been commented out don;t show up(duh!!) can someone please help me solve this problem

// YOUR NAME HERE
// Test file for assignment 1.

#include <iostream>
#include <utility>
#include <string>

#include "matrix.h"

// It is fine to use namespaces in .cc files.
using namespace std;
using namespace linear_algebra_project;

namespace {

template <typename Object>
void PrintVec(const vector<Object> &a_vector) {
for(int i=0;i<a_vector.size();i++)
cout<< a_vector[i];
}

void TestPart1() {
Matrix<int> a, b;
cout << a.NumRows() << " " << a.NumCols() << endl;

a.ReadMatrix();
cout<<"reading matix a"<<endl;

cout << a << endl;
b.ReadMatrix();
cout<<"reading matix B"<<endl;

cout << b << endl;

a = b;
cout<<"assigning matix b to a"<<endl;

cout << a << endl;


Matrix<int> c = b;
cout<<"creating c and assigning matix b"<<endl;


cout << c << endl;

Matrix<int> d = std::move(c);
cout << d << endl;
cout<<"creating d and copying matix c"<<endl;

// a = std::move(d);
cout<<" copying matix d to a" <<endl;

}

void TestPart2() {
Matrix<string> a, b;
a.ReadMatrix();
cout << a << endl;
b.ReadMatrix();
cout << b << endl;
cout << a + b << endl; // Matrices should have same size.
// The default + operator for strings
// will be used.

Matrix<string> d = a + b;
cout << d <<endl;

const string a_string="hi_there";
// cout << d + a_string << endl;

PrintVec(a[1]); // Should print the first row of a.
PrintVec(b[2]); // Should be print the second row of b.
// Note, that the [] operator should return
// a vector object.
// PrintVec() is a templated function that
// couts the elements of a vector.
}

} // namespace

int main() {
TestPart1();
TestPart2();
return 0;
}

[tpb]

whenever I comment out the lines that call built in move functions I get segmentation fault,if I comment them out everything runs smoothly

You're babbling. And your code is unreadable. Somehow it's lost its indentation.

[Thomas1965]

Please show us the code of the matrix class.

[keskiverto]

Please use code tags (and intuitive indentation) when posting code. See http://www.cplusplus.com/articles/jEywvCM9/


Could you explain to us what the std::move does (as far as you know)?

[llll]

HI
thanks guys for pointing out indentation, what I copied was supposed to be indented well, I just looked at what I pasted and I sincerely apologize,

[llll]

@Thomas1965, I am not sure if posting code for the matrix class will help because to be honest I thought there is something wrong with the other code and I looked thoroughly before I posted I could't find anything wrong everything else seem to be working fine, I am still pasting the code here


// A simple basic implementation of a Matrix class

#ifndef TEACH_CSCI335_MATRIX_H_
#define TEACH_CSCI335_MATRIX_H_

#include <iostream>
#include <vector>

using namespace std;


// Make it a habit to use namespaces for your code.
namespace linear_algebra_project {

// Templated implementation of Matrix
// Sample usage:
// Matrix<int> a;
// a.ReadMatrix();
// cout << a_matrix.NumRows() << endl;
// cout << a_matrix.NumCols() << endl;
// cout << a;
// Matrix<int> b;
// b.ReadMatrix();
// cout << a + b;
template <typename Object>
class Matrix {
public:
Matrix() {
num_rows_ = 0 ;
num_columns_=0 ;
array_= nullptr;
}

Matrix(const Matrix &rhs) {
array_=new Object *[rhs.num_rows_];
for(int i=0;i<num_rows_;i++)
{
array_[i]=new Object[num_columns_];
}
for (int i=0;i<rhs.num_rows_;i++)
{
for(int j=0;j<rhs.num_columns_;j++)
{
array_[i][j]=rhs[i][j];
}
}
num_columns_ = rhs.num_columns_;
num_rows_ = rhs.num_rows_;
}

Matrix& operator=(const Matrix &rhs) {
if (this != &rhs) {
Object **send = new Object* [rhs.num_rows_] ;
for (int i=0;i<rhs.num_rows_;i++)
{
send[i]=new Object[rhs.num_columns_];
}
for (int i=0;i<rhs.num_rows_;i++)
{
for(int j=0;j<rhs.num_columns_;j++)
{
send[i][j]=rhs.array_[i][j];
cout<<"we are copying"<<send[i][j]<<endl;
}
}
// Deallocate memory of array_;
for (int i = 0; i < num_rows_; ++i)
delete [] array_[i];
delete[] array_;
array_ = send;
num_columns_ = rhs.num_columns_;
num_rows_ = rhs.num_rows_;
}
return *this;

}

Matrix(Matrix &&rhs) {
cout << "HEY tHIS IS MOVE CONSTRUCTOR" << endl;
array_ = rhs.array_;
num_columns_ = rhs.num_columns_;
num_rows_ = rhs.num_rows_;
rhs.array_ = nullptr;
rhs.num_columns_ = 0;
rhs.num_rows_ = 0;
}

Matrix& operator=(Matrix &&rhs) {
for (int i = 0; i < num_rows_; ++i)
delete [] array_[i];
delete[] array_;
return *rhs;

}


void ReadMatrix() {
std::cin >> num_rows_;
std::cin >> num_columns_;

array_ = new Object *[num_rows_];
for (size_t i = 0; i < num_rows_; ++i)
{
array_[i] = new Object[num_columns_];
}

for(int i=0;i<num_rows_;i++)
{
for(int j=0;j<num_columns_;j++)
{
std::cin >> array_[i][j];
}
}
}
// @row: an index to a row of the matrix.
// @returns the row as a vector of items. No error checking.
// const version.
std::vector<Object> operator[](int row) const
{
vector<Object> vecs(num_columns_);
for (int j=0;j<num_columns_;j++)
{
vecs[j]=array_[row][j];
}
return vecs;
}



Matrix operator+(const Matrix &b_matrix) {
Matrix result;
result.array_ = new Object*[b_matrix.num_rows_];
for (int i=0;i<b_matrix.num_rows_;i++)
{
output][/output] result.array_[i]=new Object[b_matrix.num_columns_];
}
for (int i=0;i<b_matrix.num_rows_;i++)
{
for(int j=0;j<b_matrix.num_columns_;j++)
{
result.array_[i][j]=b_matrix.array_[i][j]+array_[i][j];
}
}
result.num_rows_ = b_matrix.num_rows_;
result.num_columns_ = b_matrix.num_columns_;
return result;
}


Matrix operator+(const Object &an_object) {
this[num_rows_+1]=new Object[num_columns_];
this[num_rows_-1][0]=an_object;

}

// @returns number of rows.
size_t NumRows() const { return num_rows_; }
// @returns number of columns.
size_t NumCols() const { return num_columns_; }

// Overloading the << operator.
friend std::ostream &operator<<(std::ostream &out, const Matrix &a_matrix) {
out << "OUTPUT OF MATRIX:" << std::endl;
for (size_t i = 0; i < a_matrix.num_rows_; ++i)
for (size_t j = 0; j < a_matrix.num_columns_; ++j)
out << a_matrix.array_[i][j] << " ";
out << std::endl;
return out;//am I returning pointer to thestream or stream object itself? confused because of return type
}

private:
size_t num_columns_;// how come its not declared as int and I still dont get error
size_t num_rows_;
Object **array_;
};

} // namespace linear_algebra_project

#endif // TEACH_CSCI335_MATRIX_H

[icy1]

Matrix(Matrix &&rhs) {
cout << "HEY tHIS IS MOVE CONSTRUCTOR" << endl;
array_ = rhs.array_;
num_columns_ = rhs.num_columns_;
num_rows_ = rhs.num_rows_;
rhs.array_ = nullptr;
rhs.num_columns_ = 0;
rhs.num_rows_ = 0;
}

Matrix& operator=(Matrix &&rhs) {
for (int i = 0; i < num_rows_; ++i)
delete [] array_[i];
delete[] array_;
return *rhs;

}

Where's your "HEY THIS IS MOVE ASSIGNMENT" ...?

Matrix<int> d = std::move(c);
...

// a = std::move(d);

...because you're doing that twice. Did you mean Matrix<int> d(std::move(c)); ? Note: [code] tags!

And your implementation of both of these looks quite scary and crash-worthy.

[Thomas1965]

The problem is in your Copy Constructor
Access violation writing location 0xCDCDCDCD
array_[i][j] = rhs[i][j];

Return value missing on Matrix operator+(const Object &an_object)

[keskiverto]

I just looked at what I pasted and I sincerely apologize

You can edit your posts to add code tags.


Why do you have a mix of raw arrays and std::vectors? Why don't you implement Matrix with std::vector?

Note:

array_ = new Object * [rows];
for ( int i=0; i<rows; i++ )
{
  array_[i] = new Object[columns];
}

Yes, that's what they all do. Raw could be:

array_ = new Object * [rows];
array_[0] = new Object [rows*cols];
for ( int r=1; r<rows; ++r )
{
  array_[r] = array_[0] + r*cols;
}

That is simpler to deallocate too:

delete [] array_[0];
delete [] array_;
array_ = nullptr;

Whatever implementation you do choose, pack the allocation and deallocation into helper functions, for you do them in multiple places and logical typos can easily creep in.

For example:

Matrix( const Matrix &rhs )
{
  array_ = new Object *[rhs.num_rows_];
  for ( int i=0; i<num_rows_; i++ ) // use of uninitialized value
  {
    array_[i] = new Object[num_columns_]; // use of uninitialized value
  }
  for ( int i=0; i<rhs.num_rows_; i++ )
  {
    for( int j=0; j<rhs.num_columns_; j++ )
    {
      array_[i][j] = rhs[i][j];  // THIS IS AWESOME *
    }
  }
  num_columns_ = rhs.num_columns_;
  num_rows_ = rhs.num_rows_;
}

* What is so mind-boggling about that?
The rhs IS-A Matrix. You do call Matrix::operator[](int) a total of rhs.num_rows_ * rhs.num_columns_ times.
Each call constructs temporary unnamed std::vector<Object>, whose operator[] you then call. Once.

The first errors you would have avoided by use of initializer list syntax:

Matrix( const Matrix &rhs )
 : num_columns_( rhs.num_columns_ ),
   num_rows_( rhs.num_rows_ ),
   array_( new Object* [rhs.num_rows_] );
{
  for ( int i=0; i<num_rows_; i++ ) // ok
  {
    array_[i] = new Object[num_columns_]; // ok
  }

  for ( int r=0; r<num_rows_; ++r )
  {
    std::copy( rhs.array_[r], rhs.array_[r] + num_columns_, array_[r] ); // fancy
  }
}

[llll]

I sincerely apologize to all I was way too occupied with school ,events in my life I hope my apologies are accepted and I truly appreciate everyone who is willing to help me out

[llll]

@Thomas I don't understand what you're saying about copy constructor I believe its alright and I followed your last suggestion still didn't help

@Keskiverto I tried using initializer list at the very beginning for the very first code for some reason it did not work for me I kept on getting errors until I did it the old fashioned way also I noticed the way I populated this object wasn't right from your code thanks

@icy1 not sure if I should report you but please reread the portion of code you copied but looking at your code also made me realize that I had implemented both =, and move constructor and probably the complier is getting confused about which one to use and what's going on

[llll]

@Keskiverto I am seriously confused about what could be the difference between Matrix(Matrix &&rhs) and Matrix(const Matrix &rhs), the only difference is in const parameter and I don't understand if there could be any difference in the code because of it, and the problem is with move function still