Clase básica de la matriz en C ++ -- ++ campo con beginner campo con matrix camp codereview Relacionados El problema

Basic Matrix Class in C++


8
vote

problema

Español

He hecho una clase simple de matriz para aprender C ++. No usa plantillas, por lo que solo funciona con [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3] 2 . Lo hice de esta manera para entender realmente cómo trabajar con objetos y memoria. Probablemente me mudaré al siguiente nivel una vez que recibo comentarios sobre este.

Utilicé estas dos publicaciones de blogs como referencia:

  • Clase de matriz C ++
  • Clase de tensor C ++

Descripción

  • Los números ( [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3] 3 ) se almacenan en una matriz 1D de tamaño fijo. Podemos remodelar la matriz, pero la nueva forma no puede modificar el tamaño total
  • Admite la matriz básica y las operaciones escalares (+, -, *)
  • Algunas funciones de ayudantes están disponibles como llenar la matriz con un valor, un rango o enteros aleatorios

Una pregunta que tengo es cuando accede a un miembro de una instancia, ¿cuál es la diferencia entre usar [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3] 4 o simplemente 99887766555443315 (si hay uno)?

Además, cuando se pasa una instancia por referencia, puedo acceder a sus miembros incluso los privados. Puede ver este comportamiento en la implementación del constructor de copia y en muchos otros lugares. Es una buena práctica? ¿O debo tener un entorno público para acceder al miembro privado de una clase, incluso cuando son el mismo tipo?

cmatrix.hpp

  [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3] 6  

cmatrix.cpp

  [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3] 7  
Original en ingles

I've made a simple Matrix class to learn C++. It doesn't use Templates so it only works with int. I made it this way to really understand how to work with objects and memory. I'll probably move to the next level once I get feedback on this one.

I used these two blog posts as reference:

  • C++ Matrix Class
  • C++ Tensor Class

Description

  • The numbers (int) are stored in a 1D array of fixed size. We can reshape the matrix but the new shape can't modify the total size
  • It supports basic matrix and scalar operations (+,-,*)
  • Some helpers functions are available like filling the matrix with one value, a range or random integers

One question I have is when I access a member of an instance, what is the difference between using this->m_member or just m_member (if there is one)?

Also, when an instance is passed by reference, I can access its members even the private ones. You can see this behavior on the implementation of the copy constructor and at many other places. Is it good practice? Or should I have a public getter for accessing the private member of a class, even when they are the same type?

CMatrix.hpp

#ifndef CMatrix_hpp #define CMatrix_hpp   #include <iostream> #include <random>  class CMatrix { private:     int* m_ptValues;     int m_totalSize;     int m_rows;     int m_columns; public:     CMatrix(int rows, int columns); // base ctor.     CMatrix(const CMatrix& rhs); // copy ctor.     CMatrix& operator=(const CMatrix& rhs); // assign. ctor.     ~CMatrix(); // dtor.     int& operator()(int row, int column);     int& operator()(int row, int column) const;     CMatrix& operator+=(int scalar);     CMatrix operator+(int scalar);     CMatrix& operator-=(int scalar);     CMatrix operator-(int scalar);     CMatrix& operator*=(int scalar);     CMatrix operator*(int scalar);     CMatrix& operator*=(const CMatrix& rhs);     CMatrix operator*(const CMatrix& rhs);     CMatrix& operator+=(const CMatrix& rhs);     CMatrix operator+(const CMatrix& rhs);     void reshape(int newRows, int newColumns);     void show(); //used for dev. only     void range(int start, int defaultStep=1);     void fill(int value);     void randint(int lowerBound, int upperBound); };  #endif /* CMatrix_hpp */ 

CMatrix.cpp

#include "CMatrix.hpp"  CMatrix::CMatrix(int rows, int columns) : m_rows(rows), m_columns(columns) {     m_totalSize = m_rows * m_columns;     m_ptValues = new int[m_totalSize](); } CMatrix::CMatrix(const CMatrix& rhs) : m_rows(rhs.m_rows), m_columns(rhs.m_columns) {     m_totalSize = rhs.m_totalSize;     m_ptValues = new int[rhs.m_totalSize]();     std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int)); } CMatrix& CMatrix::operator=(const CMatrix& rhs) {     if (&rhs == this) {         return *this;     }     if (m_totalSize == rhs.m_totalSize) {         std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int));     } else {         delete[] m_ptValues;         m_ptValues = new int[rhs.m_totalSize]();         std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int));     }     m_rows = rhs.m_rows;     m_columns = rhs.m_columns;     m_totalSize = rhs.m_totalSize;      return *this; } CMatrix::~CMatrix() {     if (m_ptValues) {         delete[] m_ptValues;     } } int& CMatrix::operator()(int row, int column) {     if (row >= m_rows || column >= m_columns) {         throw std::out_of_range("Index out of bounds");     }     return m_ptValues[row * m_columns + column]; } int& CMatrix::operator()(int row, int column) const {     if (row >= m_rows || column >= m_columns) {         throw std::out_of_range("Index out of bounds");     }     return m_ptValues[row * m_columns + column]; } CMatrix& CMatrix::operator+=(int scalar) {     for (auto i = 0; i < m_totalSize; i++) {         m_ptValues[i] += scalar;     }     return *this; } CMatrix CMatrix::operator+(int scalar) {     CMatrix result(m_rows, m_columns);      for (auto i = 0; i < m_totalSize; i++) {         result.m_ptValues[i] = m_ptValues[i] + scalar;     }     return result; } CMatrix& CMatrix::operator-=(int scalar) {     for (auto i = 0; i < m_totalSize; i++) {         m_ptValues[i] -= scalar;     }     return *this; } CMatrix CMatrix::operator-(int scalar) {     CMatrix result(m_rows, m_columns);      for (auto i = 0; i < m_totalSize; i++) {         result.m_ptValues[i] = m_ptValues[i] - scalar;     }     return result; } CMatrix& CMatrix::operator*=(int scalar) {     for (auto i = 0; i < m_totalSize; i++) {         m_ptValues[i] *= scalar;     }     return *this; } CMatrix CMatrix::operator*(int scalar) {     CMatrix result(m_rows, m_columns);      for (auto i = 0; i < m_totalSize; i++) {         result.m_ptValues[i] = m_ptValues[i] * scalar;     }     return result; } CMatrix& CMatrix::operator*=(const CMatrix& rhs) {     CMatrix result = (*this) * rhs;     (*this) = result;     return *this; } CMatrix CMatrix::operator*(const CMatrix& rhs) {     if (m_columns != rhs.m_rows) {         throw std::length_error("Matrices shapes mismatch");     }     CMatrix result(m_rows, rhs.m_columns);     int sum;     for (auto i = 0; i < m_rows; i++) {         for (auto j = 0; j < rhs.m_columns; j++) {             sum = 0;             for (auto k = 0; k < m_rows; k++) {                 sum += this->operator()(i, k) * rhs(k,j);             }             result(i,j) = sum;         }     }     return result; } CMatrix& CMatrix::operator+=(const CMatrix& rhs) {     CMatrix result = (*this) + rhs;     (*this) = result;     return *this; } CMatrix CMatrix::operator+(const CMatrix& rhs) {     if (m_rows != rhs.m_rows || m_columns != rhs.m_columns) {         throw std::length_error("Matrices shapes mismatch");     }     CMatrix result(m_rows, m_columns);     for (auto i = 0; i < m_totalSize; i++) {         result.m_ptValues[i] = this->m_ptValues[i] + rhs.m_ptValues[i];     }     return result; } void CMatrix::reshape(int newRows, int newColumns) {     if (newRows * newColumns > m_totalSize) {         throw std::invalid_argument("Total size of new matrix must be unchanged");     }     m_rows = newRows;     m_columns = newColumns; } void CMatrix::show() {     std::string delimiter = "";     for (auto i = 0; i < m_rows; i++) {         delimiter = "";         for (auto j = 0; j < m_columns; j++) {             std::cout << delimiter << m_ptValues[i * m_columns + j];             delimiter = ",";         }         std::cout << std::endl;     } } void CMatrix::range(int start, int defaultStep) {     int counter = start;     for (auto i = 0; i < m_totalSize; i++) {         m_ptValues[i] = counter;         counter += defaultStep;     } } void CMatrix::fill(int value) {     for (auto i = 0; i < m_totalSize; i++) {         m_ptValues[i] = value;     } } void CMatrix::randint(int lowerBound, int upperBound) {     std::random_device rd;     std::mt19937 mt;     std::uniform_int_distribution<int> dist(lowerBound, std::nextafter(upperBound, __DBL_MAX__));     for (auto i = 0; i < m_totalSize; i++) {         m_ptValues[i] = dist(mt);     } } 
        
     
     

Lista de respuestas

6
 
vote
vote
La mejor respuesta
 

Mirando su encabezado primero:

  #include <iostream> #include <random>   

Su interfaz no usa ninguno de los nombres definidos en estos encabezados, por lo que no es necesario incluirlos aquí. (Puede que le resulte que los necesite en la implementación, pero no son necesarios para el encabezado).


  params[:next_cards]0  

El primero de los se ve bien, pero hay un error en su segundo. Si el objeto es const, entonces no debería permitir que el código de llamadas cambie sus elementos devolviendo referencias modificables a ellos. Cualquiera de los siguientes es consistente:

  params[:next_cards]1  

Siguiente, estos métodos:

  params[:next_cards]2  

No estoy seguro de que ninguno de estos métodos pertenezca en la interfaz pública. Si proporciona tipos de iteradores adecuados, es posible que pueda subcontratar algunos o todos estos a funciones de algoritmos estándar.


Pasando al archivo de implementación, estoy un poco preocupado de que esté realizando su propia asignación de memoria y desascripción. Puede usar un 99887766655443313 para sus valores, y obtendrá su administración de memoria y copiar constructor de forma gratuita. Yo sugeriría a los miembros como

  params[:next_cards]4  

Esto le da una forma más conveniente de sus bucles:

  params[:next_cards]5  

Comprobación de rango:

  params[:next_cards]6  

Si va a hacer una comprobación de rango, también debe verificar que params[:next_cards]7 y 99887766555443318 - O le garantice, cambiando la interfaz para tomar un tipo sin firmar ( por ejemplo, params[:next_cards]9 ).


  user/next_cards/:id0  

El idioma habitual para user/next_cards/:id1 es implementarlo en términos de user/next_cards/:id2 :

  user/next_cards/:id3  

Tenga en cuenta que el objeto 998877665554433244 está const aquí: solo estamos modificando una copia .

Los otros operadores pueden seguir un patrón similar. En particular, tiene user/next_cards/:id5 y user/next_cards/:id6 al revés.


  user/next_cards/:id7  

Su mensaje de error no coincide con lo que está probando: ¿debe estar sin cambios, o simplemente no es más grande?


  user/next_cards/:id8  

Creo que puede beneficiarse de pasar la transmisión de salida como un parámetro aquí, por lo que puede (por ejemplo,) imprimir en user/next_cards/:id9 o a un archivo. Probablemente tiene sentido implementar resource :next_cards, only: :show 0 de la manera convencional en lugar de (o así como) esta función miembro. De cualquier manera, la matriz debe ser const.


Para responder a las preguntas rápidas:

  • Sí, en una función de Miembros, resource :next_cards, only: :show 11 es el mismo que resource :next_cards, only: :show 2 a menos que declare un local del mismo nombre.
  • Puede (y debe, cuando sea necesario) acceder a los miembros privados de todos los objetos del mismo tipo: los modificadores de acceso son propiedad del código, no la instancia particular que está manipulando.
 

Looking at your header first:

#include <iostream> #include <random> 

Your interface doesn't use any of the names defined in these headers, so no need to include them here. (You may find you need them in the implementation, but they are not required for the header).


int& operator()(int row, int column); int& operator()(int row, int column) const; 

The first of those looks good, but there's a mistake in your second one. If the object is const, then you shouldn't allow the calling code to change its elements by returning modifiable references to them. Either of the following is consistent:

const int& operator()(int row, int column) const; int operator()(int row, int column) const; 

Next, these methods:

void reshape(int newRows, int newColumns); void show(); //used for dev. only void range(int start, int defaultStep=1); void fill(int value); void randint(int lowerBound, int upperBound); 

I'm not sure any of these methods belong in the public interface. If you provide suitable iterator types, you may be able to outsource some or all of these to standard algorithm functions.


Moving on to the implementation file, I'm a little concerned that you're doing your own memory allocation and deallocation. You could use a std::vector<int> for your values, and you'd get your memory management and copy constructor for free. I'd suggest members like

class CMatrix { private:     std::vector<int> values;     // size is found by values.size()     int rows;     // columns is values.size()/rows } 

This gives you a more convenient form of your loops:

for (auto& i: values)     ...; 

Range checking:

int& CMatrix::operator()(int row, int column) {     if (row >= m_rows || column >= m_columns) {         throw std::out_of_range("Index out of bounds");     } 

If you're going to do range checking, you should also check that 0 <= row and 0 <= column - or guarantee it, by changing the interface to take an unsigned type (e.g. size_t).


CMatrix CMatrix::operator+(int scalar) {     CMatrix result(m_rows, m_columns);      for (auto i = 0; i < m_totalSize; i++) {         result.m_ptValues[i] = m_ptValues[i] + scalar;     }     return result; } 

The usual idiom for operator+() is to implement it in terms of operator+=():

CMatrix CMatrix::operator+(int scalar) const {     auto result = *this;     return result += scalar; } 

Note that the this object is const here: we're only modifying a copy.

The other operators can follow a similar pattern. In particular, you have * and *= the other way around.


if (newRows * newColumns > m_totalSize) {     throw std::invalid_argument("Total size of new matrix must be unchanged"); } 

Your error message doesn't match what you're testing - must it be unchanged, or just not bigger?


void CMatrix::show() {     std::string delimiter = "";     for (auto i = 0; i < m_rows; i++) {         delimiter = "";         for (auto j = 0; j < m_columns; j++) {             std::cout << delimiter << m_ptValues[i * m_columns + j];             delimiter = ",";         }         std::cout << std::endl;     } } 

I think you may benefit from passing the output stream as a parameter here, so you can (e.g.) print to cerr or to a file. It probably makes sense to implement operator<<() in the conventional manner instead of (or as well as) this member function. Either way, the matrix should be const.


To answer the quick questions:

  • Yes, in a member function, m_member is the same as this->m_member unless you declare a local of the same name.
  • You can (and should, where necessary) access the private members of all objects of the same type - the access modifiers are a property of the code, not the particular instance you're manipulating.
 
 
   
   
2
 
vote

Un punto importante aquí (a.k.a. un error ), la cantidad de filas y columnas cambia esta función:

  void CMatrix::reshape(int newRows, int newColumns) {     if (newRows * newColumns > m_totalSize) {         throw std::invalid_argument("Total size of new matrix must be unchanged");     }     m_rows = newRows;     m_columns = newColumns; }   

La función necesita actualizar también el parámetro m_totalSize :

  m_totalSize = m_rows * m_columns;   

Personalmente no tendría tal función. No creo que tenga sentido cambiar un ancho de matriz o la altura como esa .


En el operador de asignación, elimina y reasignó el búfer si no es exactamente igual, lo cual es contradictorio en comparación con que el remodelador () anterior.

El if(m_totalSize == rhs.m_totalSize) podría ser if(m_totalSize >= rhs.m_totalSize) . Pierde algunos bytes en el proceso, pero evite un 99887776655443355 + new que generalmente son lentos.

  CMatrix& CMatrix::operator=(const CMatrix& rhs) {     if (&rhs == this) {         return *this;     }     if (m_totalSize == rhs.m_totalSize) {  // replace '==' with '<='         std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int));     } else {         delete[] m_ptValues;         m_ptValues = new int[rhs.m_totalSize]();          std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int));     }     m_rows = rhs.m_rows;     m_columns = rhs.m_columns;     m_totalSize = rhs.m_totalSize;      return *this; }   

Otra cosa, con C ++ 11 y más nuevo, debe usar nullptr cuando prueba un puntero, como en:

  CMatrix::~CMatrix() {     if (m_ptValues != nullptr) {         delete[] m_ptValues;     } }   

Eliminar también es lo suficientemente inteligente como para probar para los punteros nulos, por lo que realmente podría simplificar con:

  m_totalSize0  

(que se dice, hay compiladores que se pueden configurar para eliminar esa prueba, por lo que puede ser útil mantenerlo alrededor. Algunas personas que trabajan en sistemas incrustados lo ven como una optimización.

También en su caso, ese puntero no puede ser nulo. Si un constructor no puede asignar ese búfer de memoria, lanzará un error 998877666554433111 .

Finalmente, debe usar un 99887766655443312 (¡Cuidado con el operador de copia al usar esa!) De esa manera, siempre se elimina automáticamente o se menciona en la otra respuesta, use m_totalSize3 . La mayoría de la implementación de STL le indican si su índice está fuera de límites cuando se compila en modo de depuración o use el miembro 998877766555443314 .

 

An important point here (a.k.a. a bug), the number of rows and columns is changed by this function:

void CMatrix::reshape(int newRows, int newColumns) {     if (newRows * newColumns > m_totalSize) {         throw std::invalid_argument("Total size of new matrix must be unchanged");     }     m_rows = newRows;     m_columns = newColumns; } 

The function needs to also update the m_totalSize parameter:

m_totalSize = m_rows * m_columns; 

I personally would not have such a function. I don't think that it makes sense to change a matrix width or height just like that.


In the assignment operator, you delete and reallocate the buffer if not exactly equal, which is contradictory compared that the reshape() above.

The if(m_totalSize == rhs.m_totalSize) could be if(m_totalSize >= rhs.m_totalSize). You lose some bytes in the process but avoid a delete + new which are generally slow.

CMatrix& CMatrix::operator=(const CMatrix& rhs) {     if (&rhs == this) {         return *this;     }     if (m_totalSize == rhs.m_totalSize) {  // replace '==' with '<='         std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int));     } else {         delete[] m_ptValues;         m_ptValues = new int[rhs.m_totalSize]();          std::memcpy(m_ptValues, rhs.m_ptValues, rhs.m_totalSize * sizeof(int));     }     m_rows = rhs.m_rows;     m_columns = rhs.m_columns;     m_totalSize = rhs.m_totalSize;      return *this; } 

Another thing, with c++11 and newer, you should use nullptr when you test a pointer, as in:

CMatrix::~CMatrix() {     if (m_ptValues != nullptr) {         delete[] m_ptValues;     } } 

Also delete is smart enough to test for null pointers, so really you could simplify with:

CMatrix::~CMatrix() {     delete[] m_ptValues; } 

(that being said, there are compilers that can be setup to remove that test, so it can be useful to keep it around. Some people working on embedded systems view it as an optimization.

Also in your case, that pointer can't be null. If a constructor can't allocate that memory buffer, it will throw an std::bad_alloc error.

Finally, you should use an std::unique_ptr<>() instead (watch out for the copy operator when using that one!) That way it always automatically gets deleted or as mentioned in the other answer, use std::vector<>. Most STL implementation tell you if your index is out of bounds when compiled in debug mode or you use the at() member.

 
 

Relacionados problema

1  Integración de oscilador de fase perturbada  ( Perturbed phase oscillator integration ) 
Estoy integrando un sistema de osciladores de fase perturbados. Defino el sistema de ecuación y también la matriz jacobiana. Tengo que remodelar el vector dim...

4  Aumentar el rendimiento de una matrícula de la multiplicación de matriz de 2x2  ( Increase performance of a spate of 2x2 matrix multiplication ) 
Me gustaría mejorar el rendimiento de la pieza de código a continuación (en Fortran). Da buenos resultados, pero el perfil le dice que es donde pasa la mayor ...

4  Asignación de matrices para la modificación en el lugar  ( Allocating matrices for in place modification ) 
Este código parece estar funcionando. Estoy asignando matrices en la pila y los pasa a funciones para modificar en su lugar. ¿Es esta una práctica estándar, o...

3  Análisis de simetría para arreglos atómicos en un cristal  ( Symmetry analysis for atom arrangements in a crystal ) 
Por un tiempo ahora he tenido la intención de publicar un poco de mi código Haskell aquí para que alguien pueda decirme qué partes de la biblioteca de idiomas...

0  Código de número de rutas posible de la longitud dada entre 2 nodos dados  ( Code for number of routes possible of the given length between 2 given nodes ) 
Recientemente me encontré en este problema , aquí hay un extracto de eso, Es bien sabido que el algoritmo de enrutamiento utilizado en Internet es altam...

2  La matriz hash realiza la eliminación gaussiana  ( Hash matrix performs gaussian elimination ) 
Esta es la matriz de hash que usé para procesar la matriz para mi algoritmo cuadrático de tamiz. Puede encontrar el código completo en aquí (es java 7 ), p...

2  Hackerank - Queens Attack II - Java  ( Hackerrank queens attack ii java ) 
La definición del problema ya se puede encontrar aquí . Dado un tablero de ajedrez con dimensiones N Ã- N (donde n es de hasta 100000) y el ( r , C )...

5  Multiplicación de matriz y producto de punto  ( Matrix multiplication and dot product ) 
Ejercicio 2.37. Supongamos que representamos vectores v = (vi) como secuencias de números, y matrices m = (MIJ) como secuencias de vectores (las filas ...

19  Clase de matriz en C #  ( Matrix class in c ) 
He estado aprendiendo C # durante mi tiempo libre en los últimos meses; Antes de eso, en su mayoría estaba escribiendo Java, por lo que la transición no ha si...

3  Solución más rápida para la resta sabia de la matriz  ( Faster solution for row wise matrix subtraction ) 
Tengo 2 matrices. Tengo que calcular la distancia euclidiana entre cada fila de Matrix A y cada fila de Matrix B. En la primera solución i bucle sobre las f...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos