2016-12-12 10 views
1

私が作成した行列クラスで+と=演算子の両方で演算子のオーバーロードを使用しようとしています。コンストラクタまたはデストラクタのいずれかが問題を引き起こしているか、どちらかではありません(ただし、私はそれらのそれぞれをグレーアウトしていますが、両方ともコードが機能するようです)。誰かが私にこの奇妙な振る舞いの原因を理解させるのを助けてくれますか? 3つの行列aとbとcを作成しようとすると、a = b + c;それはちょうど失敗する。コンストラクタデストラクタまたはOOPの理解

header file 
#ifndef MATRIX_H; 
#define MATRIX_H; 
using namespace std; 


enter code here 

class matrix 
{ 
friend ostream& operator<< (ostream&, matrix &); 
public: 
    matrix(); 
    matrix(int,int); //constructor 
    matrix(const matrix&);//copy constructor 
    ~matrix(); 
    int getRow(); 
    int getCol(); 
    void setRow(int); 
    void setCol(int); 
    class arr1D{    //proxy class to allow the use of [][]  operator 
     public: 
     arr1D(int* a):temp(a){} 
     int &operator[](int a){ 
      return temp[a]; 
     } 
     int *temp; 
    }; 
    arr1D operator[](int a){ 
    return arr1D(arr2[a]); 
    } 
    matrix& operator=(const matrix&); 

    matrix& operator+(const matrix&); 

protected: 
private: 
    int row , col; 
    int **arr2; 

    }; 

    #endif // MATRIX_H 

enter code here 
cpp file 
#include <iostream> 
#include "matrix.h" 

using namespace std; 

matrix::matrix() 
{ 
setCol(0); 
setRow(0); 
**arr2=0; 
} 


matrix::matrix(int x, int y) //matrix constructor creates x*y matrix and     initializes to 0's 
{ 
setCol(y); 
setRow(x); 
arr2 = new int * [getRow()]; 
if (arr2) { 
    for (int i = 0; i < getRow(); i++) { 
     arr2[i] = new int [getCol()]; 

    }; 
}; 
for (int i=0; i<getRow();i++){ 
    for (int j=0;j<getCol();j++){ 
     arr2[i][j]=0; 
    }; 
}; 
} 

matrix::matrix(const matrix &m){ //defines the copying constructor 
row=m.row; 
col=m.col; 
arr2 = new int*[row]; 
for (int i=0; i<row; i++){ 
    arr2[i] = new int[col]; 
    } 
for (int i=0; i<row; i++){ 
    for (int j=0; j<col; j++){ 
     arr2[i][j] = m.arr2[i][j]; 
    } 
} 

} 

matrix::~matrix(){ //defines the destructor 
for (int i=0; i<row; i++){ 
    delete[] arr2[i]; 
    } 
delete[] arr2; 
} 


int matrix::getRow(){ //getter for row 
return row; 
} 

int matrix::getCol(){ // getter for col 
return col; 
} 

void matrix::setRow(int x){ //setter for row 
row=x; 
} 

void matrix::setCol(int x){ //setter for col 
col=x; 
} 


ostream& operator<< (ostream& output, matrix& a){ 
    int i,j; 
    for (i=0; i < a.getRow() ; i++){ 
     for (j=0; j< a.getCol() ; j++){ 

      output << " " <<a.arr2[i][j]; 

     }; 
     output << "\n"; 
    }; 
return output; 
} 

matrix& matrix::operator=(const matrix& right) 
{ 
if (this == &right) {  // Same object? 
    return *this; 
} 
row = right.row; 
col = right.col; 
for (int i=0; i<row; i++) 
{ 
    for (int j=0; j<col; j++){ 
    arr2[i][j]=right.arr2[i][j]; 
    } 
} 
return *this ; 
} 

matrix& matrix::operator+(const matrix& right) 
{ 
int row=right.row; 
int col=right.col; 
matrix result(row,col); 
for (int i = 0; i < row; i++){ 
    for (int j = 0; j < col; j++){ 
      //cout<<"arr2[i][j]="<<arr2[i][j]<<endl; 
      //cout<<"right.arr2[i][j]="<<right.arr2[i][j]<<endl; 
     result.arr2[i][j]=(arr2[i][j] + right.arr2[i][j]); 
      //cout<<"result.arr2[i][j]="<<result.arr2[i][j]<<endl; 
    }; 
}; 
return result; 
} 
+0

cppファイルの下では、 "//コピーコンストラクタを定義しています"。おそらく私はそれが間違っていると宣言しました私は本当にここ暗いです。代入演算子(=)内の –

答えて

1

まず、他の答えは指摘したように、あなたはあなたのoperator +で一時的に参照を返しています。それは未定義の動作です。

しかし、その代わりに、この方法でoperator +を書く、何を行うべきことの代わりにoperator +=を書き、その後、順番に、operator +=の面でoperator +を書きです。プログラマは+に加えて+=matrixのために働くと期待しているので、+=を除外することは意味がありません。

operator +=の場合、現在のオブジェクトへの参照を返します。

だから我々がする必要があるすべては、operator +=operator +にコードを移動です​​:+=は、現在のマトリックスを変更するので、我々は、現在のマトリックスへの参照を返す

#include <exception> 
//... 
matrix& matrix::operator+=(const matrix& right) 
{ 
    if(row != right.row || col != right.col) 
     throw std::logic_error("Matrix not the same size"); 

    for (int i = 0; i < right.row; i++) 
    { 
     for (int j = 0; j < right.col; j++) 
      arr2[i][j] += right.arr2[i][j]); 
    } 
    return *this; 
} 

注意。また、不正な行列が+=に送信された場合に例外がスローされることにも注意してください。このIMOは合法的な返信よりも合法的なmatrixを返すよりも理にかなっています。行列のサイズが同じでない場合は、コードで行列を返すべきではありません。

は今operator ++=の観点で書くことができます。

matrix matrix::operator+(const matrix& right) 
    { 
     return matrix(*this) += right; 
    } 

は信じられないかもしれませんが、それはそれです。私たちがしたのは、一時的な行列を作成し、渡された引数で+=と呼ぶことでした。私たちは期待どおり、これを新しいマトリックスとして返します。

その他の問題は代入演算子です。コピーコンストラクタとデストラクタを作成し、代入演算子を使用せずにコピーコンストラクタを使用すると、copy/swap idiomを代入演算子の実装に使用できます。

#include <algorithm> 
    //... 
    matrix& matrix::operator=(const matrix& right) 
    { 
     matrix temp(right); 
     std::swap(temp.arr2, arr2); 
     std::swap(temp.row, row); 
     std::swap(temp.col. col); 
     return *this; 
    } 

ここで行ったことは、渡されたオブジェクトの一時的な行列を作成し、その内容を現在のオブジェクトの内容で置き換えることでした。リターン時に一時的に消滅すると、一時的にスワップアウトされた古いコンテンツが破棄されます。

このメソッドは実装が非常に簡単であるだけでなく(std::swapへの呼び出しの束)、例外的に安全です。一時的な行列を作成する際に問題が発生した場合は、thisのメンバーを壊したり変更したりすることなく、std::bad_alloc例外がスローされます。新しいメモリを割り当てる前に最初にメモリの割り当てを解除している別の答えの問題は、上記のテクニックを使用して解決されます。

+0

あなたは行列temp(右)を使用します;この操作はコピーコンストラクタを呼び出しますか?そして最後に - (* this)を参照しているのは何ですか?一時的な行列または左の演算子の行列に?おかげで再び、私はちょうど私がメモリ内で起こっていることの理解が不足していると感じる面倒について申し訳ありません。 –

+0

はい、 'matrix temp(right);を実行すると、コピーコンストラクタが呼び出されます。コピー/スワップが機能するためにはコピーコンストラクタが必要です。 '* this'は現在の行列です。 – PaulMcKenzie

+0

ありがとう、より理にかなって! –

関連する問題