Implementazione di base della matrice intera

Question

Devo scrivere CustomBasicMatrixun'implementazione di classe per il college. Ciò include operazioni di base come addizione inte CustomBasicMatrixsottrazione, trasposizione e iterazioni / const iterazioni .

Vorrei sapere se esistono convenzioni di codifica o, cosa più importante, possibilità di perdite di memoria / errori di segmentazione .

Contesto:

• L'implementazione deve essere racchiusa in uno spazio dei nomi "sys".
• L'unico file esterno in uso è "Auxiliaries.h", che contiene: "Dimension" struct, contiene solo 'width' e 'height', con funzioni di supporto aggiuntive per la stampa di una matrice.
• Invece di trasporre una funzione capovolgendo le dimensioni, ho salvato un valore booleano che indica se la matrice è trasposta, quindi quando recuperiamo il valore usiamo detto booleano per determinare se recuperiamo (i, j) o (j, i). Questo per risparmiare memoria e sovraccarico.

Grazie in anticipo!

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);
}

Modifica: Aggiunto il file di intestazione degli ausiliari. Viene fornito in modo che il file cpp non sia disponibile e puoi ignorarlo nella revisione

Ausiliari.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;
    }   
}

Answer 1

1. Invece di trasporre una funzione capovolgendo le dimensioni, ho salvato un valore booleano che indica se la matrice è trasposta, quindi quando recuperiamo il valore usiamo detto booleano per determinare se recuperiamo (i, j) o (j, i). Questo per risparmiare memoria e sovraccarico.

Sfortunatamente, questo aumenta solo le spese generali. In questo modo si ha un ramo non necessario ogni volta che si desidera semplicemente accedere a un elemento della matrice rallentando il codice e inoltre si tradurrà sicuramente in uno scarso accesso alla memoria che rallenta ulteriormente il codice.

Anche l'implementazione di operazioni come +=ignora se le matrici sono trasposte. E l'unico modo per trasporre una matrice è chiamare transpose()che fa comunque una copia.

1. int** data;

Questo non va bene. Fai un'allocazione per ogni elemento di colonna della matrice, contribuendo alla frammentazione della memoria. È preferibile archiviare l'intero array di dati come un pezzo di dati contiguo, cioè int* data;capire dove iniziano e finiscono le colonne tramite la dimensione di row. O meglio avvolgere in un puntatore intelligente senza overhead esistente std::unique_ptr<int[]> data;e per inciso non sarà necessario scrivere tutto try/catchper la deallocazione.

1. Perché nessun costruttore predefinito? Almeno abilitalo. Inoltre, qui non è assolutamente necessario rendere virtuale il distruttore: non è necessario per questa classe. Rendi virtuali i distruttori per le classi che hanno metodi polimorfici.

2. int &operator()(int row, int col); int &operator()(int row, int col) const;

Credo che tu volessi che la constversione tornasse into const int&meno int&.

1. La classe dovrebbe avere il costruttore di movimento e l'operatore di assegnazione del movimento. Questo è fondamentale ai fini della memoria.

2. Cosa fa iteratornella matrice? Su cosa itera e in quale ordine? Non è chiaro dall'intestazione. Inoltre, iteratordovrebbe iterare sulle righe invece che sugli elementi della 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);

È davvero così che vuoi usare la matrice? Mi sembra strano. Potrei capire se volevi qualcosa del genere per un'immagine ... opencv lo usa cv::Matsia per le matrici che per le immagini, ma non trovo che sia una buona scelta di design avvolgere tutto in una classe.

1. Tutte le implementazioni delle tue funzioni sono nel file cpp. Non è buono. Le funzioni non possono essere incorporate se la loro definizione è nascosta. Dovresti spostare tutte le definizioni di funzioni piccole nell'intestazione.

2. Credo che le operazioni primarie di una matrice di intervallo dinamico dovrebbero essere la moltiplicazione per vettore e il calcolo del suo inverso. Entrambi mancano. Tuttavia, non sorprende considerando che hai scritto una intmatrice di tipo invece di tipi più adatti per una matrice come floate double.

3. Sono sicuro che potresti scrivere alcune semplici funzioni come reserveo resizee semplicemente usarle invece di copiare e incollare la procedura di allocazione. Inoltre, l'assegnazione della copia eliminerà inutilmente tutti i dati anche se le dimensioni corrispondono.

4. Come parte della progettazione generale ora in C++ i visualizzatori sono piuttosto popolari e consigliati: classi che non possiedono memoria (non eliminare o allocare). Una classe MatrixViewer sarebbe molto efficiente nel concludere le cose.