Implémentation de matrice d'entiers de base
J'ai besoin d'écrire une CustomBasicMatriximplémentation de classe pour l'université. Cela inclut les opérations de base telles que intet l' CustomBasicMatrixaddition et la soustraction, la transposition et itérations / itérations const .
Je voudrais savoir s'il existe des conventions de codage ou, plus important encore, des possibilités de fuites de mémoire / défauts de segmentation .
Le contexte:
- L'implémentation doit être encapsulée dans un espace de noms "sys".
- Le seul fichier externe utilisé est "Auxiliaries.h", qui contient: la structure "Dimension", ne contient que "width" et "height", avec des fonctions d'aide supplémentaires pour l'impression d'une matrice.
- Au lieu de transposer une fonction en inversant les dimensions, j'ai enregistré une valeur booléenne indiquant si la matrice est transposée, donc lors de la récupération de la valeur, nous utilisons ledit booléen pour déterminer si nous récupérons (i, j) ou (j, i). Ceci permet d'économiser de la mémoire et des frais généraux.
Merci d'avance!
CustomBasicMatrix .h:
#include <ostream>
#include "Auxiliaries.h"
namespace sys{
class CustomBasicMatrix{
private:
int rows;
int cols;
int** data;
bool trans = false;
public:
explicit CustomBasicMatrix(Dimensions dim, int initValue = 0);
CustomBasicMatrix(const CustomBasicMatrix& other);
virtual ~CustomBasicMatrix();
CustomBasicMatrix& operator=(const CustomBasicMatrix& other);
static CustomBasicMatrix Identity(int dims);
int height() const;
int width() const;
int size() const;
CustomBasicMatrix transpose() const;
CustomBasicMatrix operator-() const;
CustomBasicMatrix& operator+=(int scalar);
CustomBasicMatrix& operator+=(const CustomBasicMatrix& rhs);
int &operator()(int row, int col);
int &operator()(int row, int col) const;
class iterator{
private:
CustomBasicMatrix* matrix;
int row;
int col;
public:
iterator(CustomBasicMatrix* matrix, int row=0, int col=0);
virtual ~iterator() = default;
iterator& operator=(const iterator& other);
int& operator*();
iterator& operator++();
const iterator operator++(int);
bool operator==(const iterator& other) const;
bool operator!=(const iterator& other) const;
};
iterator begin();
iterator end();
class const_iterator{
private:
const CustomBasicMatrix* matrix;
int row;
int col;
public:
const_iterator(const CustomBasicMatrix *matrix, int row = 0, int col = 0);
virtual ~const_iterator() = default;
const_iterator& operator=(const const_iterator& other);
const int& operator*() const;
const_iterator operator++();
const const_iterator operator++(int);
bool operator==(const const_iterator& other) const;
bool operator!=(const const_iterator& other) const;
};
const_iterator begin() const;
const_iterator end() const;
};
bool any(const CustomBasicMatrix& mat);
bool all(const CustomBasicMatrix& mat);
CustomBasicMatrix operator+(const CustomBasicMatrix& lhs, const CustomBasicMatrix& rhs);
CustomBasicMatrix operator+(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator+(int scalar, const CustomBasicMatrix& matrix);
CustomBasicMatrix operator-(const CustomBasicMatrix &lhs, const CustomBasicMatrix &rhs);
CustomBasicMatrix operator<(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator<=(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator>(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator>=(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator!=(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator==(const CustomBasicMatrix& lhs, int scalar);
std::ostream &operator<<(std::ostream &os, const CustomBasicMatrix &matrix);
}
CustomBasicMatrix .cpp:
#include "CustomBasicMatrix.h"
#include "Auxiliaries.h"
#define CBM sys::CustomBasicMatrix
CBM::CustomBasicMatrix(sys::Dimensions dim, int initValue)
{
this->rows = dim.getRow();
this->cols = dim.getCol();
this->data = new int*[this->rows];
int i;
try{
for (i = 0; i < this->rows; ++i){
this->data[i] = new int[this->cols];
}
} catch(const std::exception& e){
for (int j = 0; j < i; ++j){
delete[] this->data[j];
}
delete[] this->data;
throw e;
}
for(int i=0; i< this->rows ; i++){
for (int j = 0; j < this->cols; ++j){
this->data[i][j] = initValue;
}
}
}
CBM::CustomBasicMatrix(const CBM &other)
{
this->rows = other.rows;
this->cols = other.cols;
this->trans = other.trans;
this->data = new int*[this->rows];
int i;
try{
for (i = 0; i < this->rows; ++i){
this->data[i] = new int[this->cols];
}
} catch(const std::exception& e){
for (int j = 0; j < i; ++j){
delete[] this->data[j];
}
delete[] this->data;
throw e;
}
for(int i=0; i< this->rows ; i++){
for (int j = 0; j < this->cols; ++j){
this->data[i][j] = other.data[i][j];
}
}
}
CBM::~CustomBasicMatrix()
{
for (int i = 0; i < this->rows; ++i){
delete[] this->data[i];
}
delete[] this->data;
}
CBM &CBM::operator=(const CBM &other)
{
if(this == &other) return *this;
for (int i = 0; i < this->rows; ++i){
delete[] this->data[i];
}
delete[] this->data;
this->rows = other.rows;
this->cols = other.cols;
this->trans = other.trans;
this->data = new int*[this->rows];
int i;
try{
for (i = 0; i < this->rows; ++i){
this->data[i] = new int[this->cols];
}
} catch(const std::exception& e){
for (int j = 0; j < i; ++j){
delete[] this->data[j];
}
delete[] this->data;
throw e;
}
for(int i=0; i< this->rows ; i++){
for (int j = 0; j < this->cols; ++j){
this->data[i][j] = other.data[i][j];
}
}
return *this;
}
CBM CBM::Identity(int dims)
{
Dimensions dim = Dimensions(dims, dims);
CustomBasicMatrix ret(dim, 0);
for (int i = 0; i < dims; ++i){
ret.data[i][i] = 1;
}
return ret;
}
int CBM::height() const
{
return this->trans ? this->cols : this->rows;
}
int CBM::width() const
{
return this->trans ? this->rows : this->cols;
}
int CBM::size() const
{
return this->rows * this->cols;
}
CBM CBM::transpose() const
{
CustomBasicMatrix ret(*this);
ret.trans = !ret.trans;
return ret;
}
CBM& CBM::operator+=(int scalar)
{
for (int i = 0; i < this->rows ; ++i){
for (int j = 0; j < this->cols ; ++j){
this->data[i][j] += scalar;
}
}
return *this;
}
CBM &CBM::operator+=(const CBM &rhs)
{
for (int i = 0; i < this->rows ; ++i){
for (int j = 0; j < this->cols ; ++j){
this->data[i][j] += rhs.data[i][j];
}
}
return *this;
}
CBM CBM::operator-() const
{
CustomBasicMatrix reg(*this);
for (int i = 0; i < reg.rows ; ++i){
for (int j = 0; j < reg.cols; ++j){
reg.data[i][j] = -reg.data[i][j];
}
}
return reg;
}
int &CBM::operator()(int row, int col)
{
if(this->trans)
return this->data[col][row];
else
return this->data[row][col];
}
int &CBM::operator()(int row, int col) const
{
if(this->trans)
return this->data[col][row];
else
return this->data[row][col];
}
CBM sys::operator+(const CBM &lhs, const CBM &rhs)
{
CBM temp(lhs);
return (temp += rhs);
}
CBM sys::operator+(const CBM &lhs, int scalar)
{
CBM temp = lhs;
return (temp += scalar);
}
CBM sys::operator-(const CBM &lhs, const CBM &rhs)
{
CBM temp = lhs;
return (temp += -rhs);
}
CBM sys::operator<(const CBM& lhs, int scalar)
{
CBM res(lhs);
for (int i = 0; i < res.height() ; ++i){
for (int j = 0; j < res.width(); ++j){
res(i,j) = res(i,j) < scalar;
}
}
return res;
}
CBM sys::operator<=(const CBM& lhs, int scalar)
{
CBM res1 = lhs == scalar;
CBM res2 = lhs < scalar;
return (res1 += res2);
}
CBM sys::operator>(const CBM& lhs, int scalar)
{
CBM res(lhs);
for (int i = 0; i < res.height() ; ++i){
for (int j = 0; j < res.width(); ++j){
res(i,j) = res(i,j) > scalar;
}
}
return res;
}
CBM sys::operator>=(const CBM& lhs, int scalar)
{
CBM res1 = lhs == scalar;
CBM res2 = lhs > scalar;
return res1 += res2;
}
CBM sys::operator!=(const CBM& lhs, int scalar)
{
CBM res1 = lhs > scalar;
CBM res2 = lhs < scalar;
return res1 += res2;
}
CBM sys::operator==(const CBM& lhs, int scalar)
{
CBM res(lhs);
for (int i = 0; i < res.height() ; ++i){
for (int j = 0; j < res.width(); ++j){
res(i,j) = res(i,j) == scalar;
}
} return res;
}
CBM sys::operator+(int scalar, const CBM &matrix)
{
return matrix + scalar;
}
CBM::iterator CBM::begin()
{
return iterator (this);
}
CBM::iterator CBM::end()
{
return iterator(this, this->rows, this->cols);
}
bool sys::any(const CBM &mat)
{
for (CBM::const_iterator it = mat.begin() ; it != mat.end() ; it++){
if((*it) != 0) return true;
}
return false;
}
bool sys::all(const CBM &mat)
{
for (CBM::const_iterator it = mat.begin() ; it != mat.end() ; it++){
if((*it) == 0) return false;
}
return true;
}
CBM::const_iterator CBM::begin() const
{
return const_iterator(this);
}
CBM::const_iterator CBM::end() const
{
return const_iterator(this, this->rows, this->cols);
}
std::ostream &sys::operator<<(std::ostream &os, const CBM &matrix)
{
int *vals = new int[matrix.size()];
for (int i = 0; i < matrix.height(); ++i){
for (int j = 0; j < matrix.width(); ++j){
vals[i * matrix.width() + j] = matrix(i,j);
}
}
Dimensions dim(matrix.height(), matrix.width());
std::string res = printMatrix(vals, dim);
delete[] vals;
return os << res;
}
/************************************/
CBM::iterator::iterator(CBM *matrix, int row, int col) : matrix(matrix), row(row), col(col){}
CBM::iterator &CBM::iterator::operator=(const iterator& other)
{
if(this == &other) return *this;
this->matrix = other.matrix;
this->row = other.row;
this->col = other.col;
return *this;
}
int &CBM::iterator::operator*()
{
return this->matrix->operator()(this->row, this->col);
}
CBM::iterator &CBM::iterator::operator++()
{
this->col++;
this->row += this->col / this->matrix->cols;
this->col = this->col % this->matrix->cols;
if(this->row == this->matrix->rows || this->col == this->matrix->cols){
this->row = this->matrix->rows;
this->col = this->matrix->cols;
}
return *this;
}
const CBM::iterator CBM::iterator::operator++(int)
{
iterator i = (*this);
++(*this);
return i;
}
bool CBM::iterator::operator==(const CBM::iterator &other) const
{
bool matrixEquals = (this->matrix) == (other.matrix);
bool colsEquals = this->col == other.col;
bool rowsEquals = this->row == other.row;
return matrixEquals && colsEquals && rowsEquals;
}
bool CBM::iterator::operator!=(const CBM::iterator &other) const
{
return !this->operator==(other);
}
/************************************/
CBM::const_iterator::const_iterator(const CustomBasicMatrix *matrix, int row, int col) : matrix(matrix), row(row), col(col){}
CBM::const_iterator &CBM::const_iterator::operator=(const CBM::const_iterator &other)
{
if(this == &other) return *this;
this->matrix = other.matrix;
this->row = other.row;
this->col = other.col;
return *this;
}
const int &CBM::const_iterator::operator*() const
{
return this->matrix->operator()(this->row, this->col);
}
CBM::const_iterator CBM::const_iterator::operator++()
{
this->col++;
this->row += this->col / this->matrix->cols;
this->col = this->col % this->matrix->cols;
if(this->row == this->matrix->rows || this->col == this->matrix->cols){
this->row = this->matrix->rows;
this->col = this->matrix->cols;
}
return *this;
}
const CBM::const_iterator CBM::const_iterator::operator++(int)
{
const_iterator i = (*this);
++(*this);
return i;
}
bool CBM::const_iterator::operator==(const CBM::const_iterator &other) const
{
bool matrixEquals = (this->matrix) == (other.matrix);
bool colsEquals = this->col == other.col;
bool rowsEquals = this->row == other.row;
return matrixEquals && colsEquals && rowsEquals;
}
bool CBM::const_iterator::operator!=(const CBM::const_iterator &other) const
{
return !this->operator==(other);
}
Edit: fichier d'en-tête auxiliaire ajouté. Il est fourni pour que le fichier cpp ne soit pas disponible et vous pouvez l'ignorer dans la revue
Auxiliaires.h:
#include <iostream>
#include <string>
#include <cmath>
namespace sys {
typedef int units_t;
class Dimensions {
int row, col;
public:
Dimensions( int row_t, int col_t);
std::string toString() const;
bool operator==(const Dimensions& other) const;
bool operator!=(const Dimensions& other) const;
int getRow() const ;
int getCol() const ;
};
std::string printMatrix(const int* matrix,const Dimensions& dim);
template<class ITERATOR_T>
std::ostream& printMatrix(std::ostream& os,ITERATOR_T begin,
ITERATOR_T end, unsigned int width){
unsigned int row_counter=0;
for (ITERATOR_T it= begin; it !=end; ++it) {
if(row_counter==width){
row_counter=0;
os<< std::endl;
}
os <<*it<<" ";
row_counter++;
}
os<< std::endl;
return os;
}
}
Réponses
-
Au lieu de transposer une fonction en inversant les dimensions, j'ai enregistré une valeur booléenne indiquant si la matrice est transposée, donc lors de la récupération de la valeur, nous utilisons ledit booléen pour déterminer si nous récupérons (i, j) ou (j, i). Ceci permet d'économiser de la mémoire et des frais généraux.
Malheureusement, cela ne fait qu'augmenter les frais généraux. De cette façon, vous avez une branche inutile chaque fois que vous voulez simplement accéder à un élément de la matrice ralentissant le code et en plus cela entraînera sûrement un mauvais accès à la mémoire qui ralentira encore plus le code.
De plus, la mise en œuvre d'opérations comme +=ignore si les matrices sont transposées. Et la seule façon de transposer une matrice est d'appeler transpose()ce qui fait une copie de toute façon.
-
int** data;
Ce n'est pas bon. Vous faites une allocation pour chaque élément de colonne de la matrice - contribuant ainsi à la fragmentation de la mémoire. Il est préférable de stocker l'ensemble du tableau de données en tant qu'élément de données contigu, c'est-à-dire que int* data;vous déterminez où les colonnes commencent et se terminent via la taille de row. Ou mieux envelopper dans un pointeur intelligent existant sans frais généraux std::unique_ptr<int[]> data;et en passant, vous n'aurez pas besoin d'écrire tout le try/catchpour la désallocation.
Pourquoi pas de constructeur par défaut? Au moins, activez-le. De plus, il n'est absolument pas nécessaire ici de rendre le destructeur virtuel - ce n'est pas nécessaire pour cette classe. Rendez les destructeurs virtuels pour les classes qui ont des méthodes polymorphes.
-
int &operator()(int row, int col);int &operator()(int row, int col) const;
Je crois que vous vouliez que la constversion revienne intou const int&non int&.
La classe doit avoir un constructeur de déplacement et un opérateur d'affectation de déplacement. Ceci est crucial à des fins de mémoire.
Que fait le
iteratordans la matrice? Sur quoi itère-t-il et dans quel ordre? Ce n'est pas clair dans l'en-tête. En outre, leiteratordevrait itérer sur des lignes au lieu d'éléments de la matrice.
CustomBasicMatrix operator<(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator<=(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator>(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator>=(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator!=(const CustomBasicMatrix& lhs, int scalar);
CustomBasicMatrix operator==(const CustomBasicMatrix& lhs, int scalar);
Est-ce vraiment ainsi que vous souhaitez utiliser la matrice? Cela me semble étrange. Je pourrais comprendre si vous vouliez quelque chose comme ça pour une image ... opencv l'utilise cv::Matà la fois pour les matrices et les images, mais je ne trouve pas que ce soit un bon choix de conception pour tout envelopper via une classe.
Toutes vos implémentations de fonction sont dans le fichier cpp. Ce n'est pas bon. Les fonctions ne peuvent pas être insérées si leur définition est masquée. Vous devez déplacer toutes les définitions de petites fonctions vers l'en-tête.
Je crois que les opérations principales d'une matrice de plage dynamique devraient être la multiplication par le vecteur et le calcul de son inverse. Les deux manquent. Cependant, ce n'est pas surprenant étant donné que vous avez écrit une
intmatrice de types au lieu de types plus appropriés pour une matrice commefloatetdouble.Je suis sûr que vous pouvez écrire quelques fonctions simples comme
reserveouresizeet simplement les utiliser au lieu de copier-coller la procédure d'allocation. En outre, l'attribution de copie supprimera inutilement toutes les données, même si les dimensions correspondent.Dans le cadre de la conception générale, les visualiseurs C ++ sont maintenant très populaires et recommandés - des classes qui ne possèdent pas de mémoire (ne supprimez pas ou n'allouez pas). Une classe MatrixViewer serait très efficace pour encapsuler les choses.