Des retours sur mon implémentation d'une classe Matrix

Bonjour à vous !

Je tenais d'abord à remercier les participants de ce forum pour toute l'entraide dont ils font part tout les jours.
J'essaye aussi souvent que faire se peut d' apporter aussi mon aide. C'est la première fois, de mémoire, que je demande un avis sur mon travail sur dvp.com.

Passons au vif du sujet : j'implémente depuis plusieurs semaines une classe Matrix, comme ça pour le fun ; une classe Matrix qui n'a pas pour but de concurrencer OpenBLAS ou Eigen au niveau des perfs, mais qui tient tout de même la route au niveau des performances. Je n'étais pas prêt à faire des compromis au niveau de la compréhension du code et n'est donc pas, volontairement, ajouté l'utilisation des registres SSE/AVX, ni de choses trop... exotiques.
Je souhaitais juste une bibliothèque facile à lire, qui gère tout contenu numérique, Coplien et qui gère un maximum d'opérations sur les matrices.
Il me reste encore :
- Inverse d'une matrice
- Déterminant d'une matrice
- Proposer une transposition inplace qui ne crée pas de nouvelle matrice

Hier soir je suis passé en mode template, avec un tpp, mais j'ai préféré tout rajouter dans le header, par commodité uniquement (désolé mais avec les templates je ne m'en sortais plus, beaucoup font cela apparemment, juste un .h)

Voici le code (bonne lecture).

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403

/* * Created by N.Richard * Date de création : 5 mai 2014 * Date de version : 14 mai 2014 * Version 1.0 * Principe : Bibliothèque simple pour la gestion de matrices (forme coplien) */   #ifndef MATRIX_H #define MATRIX_H   #include "Utilities.h" #ifdef _OPENMP #include <omp.h> #endif //_OPENMP #include <iostream> #include <stdexcept> #include <string.h>   template <typename T> class Matrix {   public: //Constructors Matrix (int nbrows, int nbcolumns): _rows(nbrows), _cols(nbcolumns), _values(NULL) { if (nbrows <= 0 || nbcolumns <= 0) throw std::domain_error("Matrix can't have a zero size"); _values = new T[nbrows * nbcolumns](); }   Matrix(const Matrix& mat) : _rows(mat.getNbRows()), _cols(mat.getNbCols()), _values(NULL) {   if (mat._values != NULL) { _values = new T[_rows*_cols](); memcpy(_values,mat._values, _rows*_cols * sizeof(T)); } }   //Operators override Matrix& operator= (const Matrix &mat) { if (this != &mat){   Matrix tmp (mat);   _rows = tmp._rows; _cols = tmp._cols; std::swap(_values, tmp._values); } return *this; }   bool operator== (const Matrix& mat) { if ((_rows != mat._rows) || (_cols != mat._cols)) return false;   for(int i=0; i < _rows*_cols ; ++i) if (!equal(_values[i], mat._values[i])) return false;   return true; }   bool operator!= (const Matrix& mat) const { return !(*this == mat); }   bool operator!() const { return isNull(); }   Matrix operator~() const { return transpose(); }   T& operator() (int nbrows, int nbcolumns) { #ifndef NDEBUG if (nbrows >= _rows || nbcolumns >= _cols || nbrows < 0 || nbcolumns < 0) throw std::out_of_range("const Matrix subscript out of bounds"); #endif //NDEBUG return _values[_cols*nbrows + nbcolumns]; }   T operator() (int nbrows, int nbcolumns) const { #ifndef NDEBUG if (nbrows >= _rows || nbcolumns >= _cols || nbrows < 0 || nbcolumns < 0) throw std::out_of_range("const Matrix subscript out of bounds"); #endif //NDEBUG return _values[_cols*nbrows + nbcolumns]; }   Matrix transpose() const { Matrix transposeMat (_cols, _rows);   for(int i=0 ; i < _cols ; ++i) for(int j=0 ; j < _rows ; ++j) transposeMat.set(i,j,at(j,i));   return transposeMat; }   Matrix add(const Matrix& mat) const { if(_cols != mat._cols || _rows != mat._rows) throw std::domain_error("Matrix type not compatible for addition. Both matrices must have the same number of rows and columns");   Matrix addMat (_rows, _cols);   #ifdef _OPENMP #pragma omp parallel for #endif //_OPENMP for (int i = 0; i < _rows; ++i) for (int j = 0; j < _cols; ++j) addMat(i,j) = at(i,j) + mat(i,j);   return addMat; }   Matrix mult(const Matrix& mat) const { if(_cols != mat._rows || _rows != mat._cols) throw std::domain_error("Matrix type not compatible for multiplication");   Matrix multMat (_rows, _cols);   #ifdef _OPENMP #pragma omp parallel for #endif //_OPENMP for (int i = 0; i < _rows; ++i) for(int k = 0; k < _rows; ++k) for(int j = 0; j < _cols; ++j) multMat(i,j) += at(i,k) * mat(k,j);   return multMat; }   template <typename NumericType> Matrix mult (const NumericType op) const { Matrix multMat (_rows, _cols);   #ifdef _OPENMP #pragma omp parallel for #endif //_OPENMP for (int i = 0; i < _rows; ++i) for(int j = 0; j < _cols; ++j) multMat(i,j) += at(i,j) * op;   return multMat; }   Matrix pow (const int p) const { if(!isSquare()) throw std::domain_error("Matrix type not compatible for pow operation. Matrix must be square");   if (p == 0) return Matrix::identity (_rows, _cols); else if (p == 1) return *this; else if (p%2 == 0){ Matrix powmat(_rows, _cols, false); powmat = pow(p/2); return powmat*powmat; } else if (p%2 == 1){ Matrix powmat(_rows, _cols, false); powmat = pow (p/2); return powmat*powmat**this; } else throw std::domain_error("For the moment, this library don't manage negative or float pow. As soon as possible !");   return *this; }   static Matrix identity(const int nbrows, const int nbcolumns) { if(nbrows != nbcolumns) throw std::domain_error("Matrix type not compatible for identity. Matrix must be square");   Matrix identityMat (nbrows, nbcolumns);   for (int i = 0; i < nbrows; ++i) identityMat.set(i, i, static_cast<T>(1));   return identityMat; }   void clear(T value = 0) { for(int i=0 ; i < _cols ; ++i) for(int j=0 ; j < _rows ; ++j) set(i,j,value); }   //Getters & Setters T at(int i, int j) const { return operator()(i,j); }   void set(const int i, const int j, T value) { operator()(i,j) = value; }   int getNbRows() const { return _rows; }   int getNbCols() const { return _cols; }   static int getCoutWidth() { return Matrix::COUTWIDTH; }   static void setCoutWidth(const int width) { Matrix::COUTWIDTH = width+1; }   bool isSquare () const { return (_cols == _rows); }   bool isIdentity() const { if (!isDiagonal()) return false;   for(int i=0; i < _rows; ++i) if(!equal(at(i,i),1)) return false;   return true; }   bool isDiagonal() const { if (!isSquare()) return false;   for(int i=0; i < _rows; ++i) for(int j=0; j < _cols; ++j) if (i!=j && !equal(at(i,j),0)) return false;   return true; }   bool isNull() const { for(int i=0; i < _rows; ++i) for(int j=0; j < _cols; ++j) if (!equal(at(i,j), 0)) return false;   return true; }   bool isUpperTriangular() const { if (!isSquare()) return false;   for(int i = 1; i < _rows; ++i) for (int j = 0; i != j && j < _cols ; ++j) if (!equal(at(i,j), 0)) return false;   return true; }   bool isLowerTriangular() const { if (!isSquare()) return false;   for(int i = 0; i < _rows; ++i) for (int j = i+1; j < _cols ; ++j) if (!equal(at(i,j), 0)) return false;   return true; }   //Matrix display void show () const { std::cout <<*this; }   //Destructor ~Matrix() { delete[] _values; }   static int COUTWIDTH;   private: int _rows, _cols; T* _values;   };   template<typename T> int Matrix<T>::COUTWIDTH = 8;   template<typename T, typename NumericType> Matrix<T> operator* (const NumericType op, const Matrix<T>& mat) { return mat.mult(op); }   template<typename T, typename NumericType> Matrix<T> operator* (const Matrix<T>& mat, const NumericType op) { return mat.mult(op); }   template<typename T, typename U> Matrix<T> operator* (const Matrix<T>& mat1, const Matrix<U>& mat2) { return mat1.mult(mat2); }   template<typename T, typename NumericType> Matrix<T>& operator*= (const NumericType op, Matrix<T>& mat) { return (mat = mat.mult(op)); }   template<typename T, typename NumericType> Matrix<T>& operator*= (Matrix<T>& mat,const NumericType op) { return (mat = mat.mult(op)); }   template<typename T, typename U> Matrix<T>& operator*= (Matrix<T>& mat1, const Matrix<U>& mat2) { return (mat1 = mat1.mult(mat2)); }   template<typename T, typename U> Matrix<T> operator+ (const Matrix<T>& mat1, const Matrix<U>& mat2) { return mat1.add(mat2); }   template<typename T, typename U> Matrix<T>& operator+= (Matrix<T>& mat1, const Matrix<U>& mat2) { return (mat1 = mat1.add(mat2)); }   template <typename T> std::ostream& operator << (std::ostream& out, const Matrix<T>& mat) { out <<"Matrix " <<mat.getNbRows() <<"x" <<mat.getNbCols() <<std::endl;   for(int i=0 ; i <= mat.getNbCols()*Matrix<T>::COUTWIDTH+1; ++i) out <<"="; out <<std::endl;   for(int i=0 ; i < mat.getNbRows(); ++i) { out <<"|"; out.width(Matrix<T>::COUTWIDTH); out <<mat(i, 0);   for(int j=1 ; j < mat.getNbCols(); ++j) { out.width(Matrix<T>::COUTWIDTH); out << std::right <<mat(i,j); } out <<"|" <<std::endl; }   return out; }   #endif //MATRIX_H

---

J'écouterai toutes les remarques constructives. N'hésitez surtout pas.
Je sais que beaucoup verront des choses qui leurs feront saigner les yeux. C'est justement ça que je veux corriger ;)
Merci à tous et à toutes

Salut et bravo pour ton code, l'idée est intéressante.

Quelques suggestions, en vrac:
- Jette un oeil à OpenCV, qui possède une classe cv::Mat hyper complète, pour des suggestions de fonctionnalités.

- Pour des matrices de petite taille (genre 3x3 ou 4x3) dont les dimensions sont connues à la compilation, tu pourrais aussi proposer un template similaire mais sans allocation mémoire dynamique.
ça revient un peu à remplacer :

Code:

---

1 2 3 4 5

class Matrix { ... T* _values; };

---

par

Code:

---

1 2 3 4 5

class Matrix { ... T _values[width*height]; };

---

Un peu à la manière du nouveau type std::array en C++11.
Un des avantages c'est que tu évites une allocation mémoire dynamique, ce qui peut être coûteux.

- Je ne sais pas quels sont tes besoins, mais tu pourrais proposer des itérateurs pour se balader dans la matrice. Tu pourrais même définir plusieurs types d'itérateurs, pour des accès dans un ordre particulier ou par exemple juste un sous-ensemble (une ligne ou une colonne).

Hello,

Citation:

---

Envoyé par Spleeen

Je n'étais pas prêt à faire des compromis au niveau de la compréhension du code et n'est donc pas, volontairement, ajouté l'utilisation des registres SSE/AVX, ni de choses trop... exotiques.

---

Rhoo, c'est le plus intéressant pourtant. :D

2/3 commentaires :
Effectivement une version sans allocation dynamique pour les matrices de petites tailles pourrait être un plus.

Il y à quelques variables qui ont des noms qui prêtent à confusion

Code:

---

1 2 3 4 5 6 7 8

Matrix mult(const Matrix& mat) const { if(_cols != mat._rows || _rows != mat._cols) throw std::domain_error("Matrix type not compatible for multiplication");   Matrix mult (_rows, _cols); ... }

---

Nommer une variable mult, dans une fonction mult :aie:
En survolant le code, j'ai cru à un appel de fonction.

Permettre des opérations sur des types compatibles (ou au minimum la conversion) peut être utile aussi

Code:

---

1 2 3 4 5 6 7 8

typedef Matrix<float> matf; typedef Matrix<double> matd;   matf f(4, 4); matd d(4, 4);   auto m = f * d; // devrait être possible et retourner une matd auto m = matd(f) * d; // ou éventuellement comme ça

---

Petit détail, le #include <omp.h> devrait lui aussi être entouré de #ifdef _OPENMP / #endif.

Et ton operateur<< devrait appeler show. ;)

Bonjour,

La fonction show() determine UN moyen d'affichage, le mieux serait, comme l'a dit phi1981, d'utiliser des itérateurs pour séparer le system d'affichage de la matrice.

L'operateur d'égalité pourrait ce contenter d'une boucle pour comparer les tableaux.

Citation:

---

Code:

---

1 2 3 4

class Matrix { ... T _values[width*height]; };

---

---

Il possible de passer la taille en paramètre de template.

Code:

---

1 2	template<typename Type, size_t width, size_t height> class Matrice{ .... };

---

Mais cela implique de connaître la taille à la compilation.

Salut !

C’est courageux de jeter son code en pâture aux loups qui vont s’empresser de se jeter dessus pour émettre de très nombreuses critiques, en tout cas, félicitations pour ta démarche !

Et sinon, puisqu’on parle de critiques, en voici quelques unes :

- c’est un choix de design, mais je trouve qu’il y a beaucoup trop de fonctions membres. Des fonctions comme « transpose » n’ont pas de raison d’être membres. Idem pour mult, add, etc, qui sont membres mais pourraient plutôt être statiques ou libres.
- Ta méthode identity a un paramètre en trop : tu forces l’égalité entre les deux paramètres au runtime : autant ne garder qu’un seul paramètre, ça t’évitera des erreurs et le contrat sera le même.
- Dans un autre domaine, il me semble qu’il est interdit (enfin, déconseillé) d’utiliser des noms de variables commençant par _, mieux vaut donc le mettre à la fin, ou le faire précéder de m : m_cols, m_rows…
- L’implémentation d’operator= pourrait vérifier que le buffer actuel est assez grand pour contenir l’autre matrice et ne pas réallouer dans ce cas (si on travaille sur des grosses matrices, en allouer une temporaire n’est pas forcément une bonne idée, surtout si on peut s’en passer).
- L’opérateur == devrait, pour des raisons de perfs, utiliser memcmp, ou en tout cas travailler directement sur le buffer sans passer par les operator(x,y) qui vont recalculer l’index à chaque fois ce qui est inutile.
- les opérateurs () ont des paramètres mal nommés.
- là encore, c’est un choix, mais à mon sens, appeler isDiagonal() sur une matrice non carrée ne doit pas renvoyer faux car c’est une violation de contrat --> exception puisque c’est comme ça que tu l’as géré ailleurs.
- ta matrice devrait être itérable (idéalement, de différentes manières).

Et bien sûr, tout ce qui a déjà été dit par les autres ;).

Je rajouterai:
- Les opérateurs binaires devraient être libres (même si pour les matrices il n'y devrait pas y avoir de grosse différence), et const-correct!
- l'opérateur d'affectation n'a pas à tester l'auto-affectation : tu impactes les performances pour rien (il est rare que l'on écrive a=a; )
- Les tests de bornes devraient être des assertions. Elles sont faites pour tester les erreurs de programmation. Elles sont bien plus efficaces à ce jeu que la programmation défensive.
- J'aurai posé pour invariant de la classe que (m!)._values n'est jamais nul.
- J'aurai défini un fonction offset, cela sert toujours.
- Plein de fonctions (add, mult, ..) qui ne devraient pas exister. Si tu les as déjà croisées, c'est la faute à des cours qui prennent trop leur temps pour présenter la surcharge des opérateurs.

Citation:

---

Envoyé par phi1981

Salut et bravo pour ton code, l'idée est intéressante.

---

Merci beaucoup ! Je suis un amoureux du beau code et surtout du C/C++ ; j'essaye de m'imprégner de toutes les bonnes idées et plus j'en apprends dessus plus je me rends compte du chemin astronomique qu'il me reste ^^'

Citation:

---

Envoyé par phi1981

Quelques suggestions, en vrac:
- Jette un oeil à OpenCV, qui possède une classe cv::Mat hyper complète, pour des suggestions de fonctionnalités.

---

Mon frère connait très bien les méandres d'OpenCV, je vais lui demander !

Citation:

---

Envoyé par phi1981

- Pour des matrices de petite taille (genre 3x3 ou 4x3) dont les dimensions sont connues à la compilation, tu pourrais aussi proposer un template similaire mais sans allocation mémoire dynamique.
ça revient un peu à remplacer :

Code:

---

1 2 3 4 5

class Matrix { ... T* _values; };

---

par

Code:

---

1 2 3 4 5

class Matrix { ... T _values[width*height]; };

---

Un peu à la manière du nouveau type std::array en C++11.
Un des avantages c'est que tu évites une allocation mémoire dynamique, ce qui peut être coûteux.

- Je ne sais pas quels sont tes besoins, mais tu pourrais proposer des itérateurs pour se balader dans la matrice. Tu pourrais même définir plusieurs types d'itérateurs, pour des accès dans un ordre particulier ou par exemple juste un sous-ensemble (une ligne ou une colonne).

---

Oui effectivement mais là je vois pas comment m'y prendre à part créer des classes Matrix3f, Matrix4f, Matrix3i, etc... ce qui peut être très laborieux (et pas forcément sexy je trouve).
Je sèche complètement sur les itérateurs. Je n'ai clairement pas l'habitude de les utiliser, je veux bien un lien.

Citation:

---

Envoyé par Iradrille

Rhoo, c'est le plus intéressant pourtant.

---

C'est ajouter à la TODOLIST une fois que j'aurai une version satisfaisante esthétiquement ;)

Citation:

---

Envoyé par Iradrille

Il y à quelques variables qui ont des noms qui prêtent à confusion

---

C'est corrigé, effectivement ;)

Citation:

---

Envoyé par Iradrille

Code:

---

1 2 3 4 5 6 7 8

typedef Matrix<float> matf; typedef Matrix<double> matd;   matf f(4, 4); matd d(4, 4);   auto m = f * d; // devrait être possible et retourner une matd auto m = matd(f) * d; // ou éventuellement comme ça

---

---

Est-ce que cela suffit ?
template<typename T, typename U>
Matrix<T> operator* (const Matrix<T>& mat1, const Matrix<U>& mat2)
{
return mat1.mult(mat2);
}

Citation:

---

Envoyé par Iradrille

Petit détail, le #include <omp.h> devrait lui aussi être entouré de #ifdef _OPENMP / #endif.

---

Ajouté !

Citation:

---

Envoyé par Iradrille

Et ton operateur<< devrait appeler show.

---

J'ai fait l'inverse : show est spécialisé pour le renvoyé dans la sortir standard.
L'erreur était dû à un qq ajouts antérieur dans show ^^

Citation:

---

Envoyé par PilloBuenaGente

L'operateur d'égalité pourrait ce contenter d'une boucle pour comparer les tableaux.

---

CHECK ! Même si, j'ai oublié de préciser que j'étais en -O3 (déroulement de boucles) avec toutes les options sympathiques de g++.

Citation:

---

Envoyé par PilloBuenaGente

d'utiliser des itérateurs pour séparer le system d'affichage de la matrice.

---

Je ne vois pas trop.

Citation:

---

Envoyé par PilloBuenaGente

Il possible de passer la taille en paramètre de template.

---

Ces paramètres deviendront obligatoire si je fais ça. Je peux les créer moi-même (réponse précédente) à ce moment-là ?

Merci vraiment pour tous vos retours ! :mrgreen:

Je corrige le code de mon premier post en prenant en compte vos avis. Histoire que l'étudiant passant sur mon post trouve directement de quoi le sustenter.

Citation:

---

Envoyé par Spleeen

Est-ce que cela suffit ?

Code:

---

1 2 3 4 5

template<typename T, typename U> Matrix<T> operator* (const Matrix<T>& mat1, const Matrix<U>& mat2) { return mat1.mult(mat2); }

---

---

Presque, il faut trouver le meilleur type commun entre T et U. ;)
Avec ça Matrix<float> * Matrix<double> donne un résultat Matrix<float>, alors que Matrix<double> * Matrix<float> donnera une Matrix<double>.

Pour les matrices de tailles fixes, tu peux avoir 2 classes

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18

template <class T, unsigned W, unsigned H> class SmallMatrix { };   template <class T> class LargeMatrix { };   typedef SmallMatrix<float, 4, 4> mat4; typedef SmallMatrix<float, 3, 3> mat3; typedef SmallMatrix<float, 4, 3> mat4x3; typedef SmallMatrix<float, 3, 4> mat3x4;   typedef SmallMatrix<uint32_t, 4, 4> mat4u; typedef SmallMatrix<uint32_t, 3, 3> mat3u; // ...   typedef LargeMatrix<float> mat; typedef LargeMatrix<uint_32t> matu; // ...

---

Avec l'héritage privé ou des macros, il y à moyen de factoriser pas mal de code.

J'ai attendu d'avoir tout lu pour vous répondre à tous, j'y tiens, comme vous avez eu la gentillesse d'apporter vos avis.

Citation:

---

Envoyé par white_tentacle

Salut !

C’est courageux de jeter son code en pâture aux loups qui vont s’empresser de se jeter dessus pour émettre de très nombreuses critiques, en tout cas, félicitations pour ta démarche !

---

Merci ! Oui c'est le but ; puis des claques j'en ai pris dans ma vie, c'est formateur x)

Citation:

---

Envoyé par white_tentacle

- c’est un choix de design, mais je trouve qu’il y a beaucoup trop de fonctions membres. Des fonctions comme « transpose » n’ont pas de raison d’être membres. Idem pour mult, add, etc, qui sont membres mais pourraient plutôt être statiques ou libres.

---

Va pour un static transposed (Matrix) par contre je vois pas l'intérêt pour les autres je ne vois pas tellement l'intérêt de les mettre static. Libre ?

Citation:

---

Envoyé par white_tentacle

- Ta méthode identity a un paramètre en trop : tu forces l’égalité entre les deux paramètres au runtime : autant ne garder qu’un seul paramètre, ça t’évitera des erreurs et le contrat sera le même.

---

Très juste ! La tête dans le guidon...tout ça... mais effectivement.

Citation:

---

Envoyé par white_tentacle

- Dans un autre domaine, il me semble qu’il est interdit (enfin, déconseillé) d’utiliser des noms de variables commençant par _, mieux vaut donc le mettre à la fin, ou le faire précéder de m : m_cols, m_rows…

---

Les underscores à la fin ça me fait plutôt penser à qqch de deprecated. Un très grand nombre de lib utilisent des "_" en début. Mais bon un 'm' ne coutent rien.

Citation:

---

Envoyé par white_tentacle

- L’implémentation d’operator= pourrait vérifier que le buffer actuel est assez grand pour contenir l’autre matrice et ne pas réallouer dans ce cas (si on travaille sur des grosses matrices, en allouer une temporaire n’est pas forcément une bonne idée, surtout si on peut s’en passer).

---

C'est exactement des idées comme celle-ci que j'attends, je rajoute à ma TODOLIST. Par contre, est-ce judicieux de désallouer la mémoire en trop dans ce cas ? Dois-je procéder via un realloc(_values,freespace*sizeof(T)); ?

Citation:

---

Envoyé par white_tentacle

- L’opérateur == devrait, pour des raisons de perfs, utiliser memcmp, ou en tout cas travailler directement sur le buffer sans passer par les operator(x,y) qui vont recalculer l’index à chaque fois ce qui est inutile.

---

Excellente idée. Je ne connaissais pas. Un héritage de l'assembleur ^^

Code:

---

1 2	if (memcmp (m_values, mat.m_values, sizeof(*this)) != 0) return false;

---

Ça ira ?

Citation:

---

Envoyé par white_tentacle

- les opérateurs () ont des paramètres mal nommés.

---

Un problème de fonction replace de Sublime Text ^^. Corrigé !

Citation:

---

Envoyé par white_tentacle

- là encore, c’est un choix, mais à mon sens, appeler isDiagonal() sur une matrice non carrée ne doit pas renvoyer faux car c’est une violation de contrat --> exception puisque c’est comme ça que tu l’as géré ailleurs.

---

Oui ça semble plutôt cohérent effectivement. C'est corrigé !

Citation:

---

Envoyé par white_tentacle

- ta matrice devrait être itérable (idéalement, de différentes manières).

---

Là je demande de l'aide. Les itérateurs C++ et moi ça fait deux !

Citation:

---

Envoyé par Luc Hermitte

- Les opérateurs binaires devraient être libres (même si pour les matrices il n'y devrait pas y avoir de grosse différence), et const-correct!

---

C'est vrai que j'ai fait ça pour certains et pas pour d'autres, je vais m'en occupé ! const-correct tient, j'en ai oublié ? Je dois pas avoir un retour const hein, c'est exprès ! :o

Citation:

---

Envoyé par Luc Hermitte

- l'opérateur d'affectation n'a pas à tester l'auto-affectation : tu impactes les performances pour rien (il est rare que l'on écrive a=a; )

---

Oui effectivement y'a eu débat là-dessus, pour faire un compromis voilà ma solution :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17

//Operators override Matrix& operator= (const Matrix &mat) { #ifndef NDEBUG if (this != &mat){ #endif //NDEBUG Matrix tmp (mat);   m_rows = tmp.m_rows; m_cols = tmp.m_cols; std::swap(m_values, tmp.m_values); #ifndef NDEBUG } #endif //NDEBUG   return *this; }

---

Citation:

---

Envoyé par Luc Hermitte

Les tests de bornes devraient être des assertions. Elles sont faites pour tester les erreurs de programmation. Elles sont bien plus efficaces à ce jeu que la programmation défensive.

---

Je me renseigne là-dessus, mais quel avantage face aux duos ici présent de #define et d'exceptions.

Citation:

---

Envoyé par Luc Hermitte

- J'aurai posé pour invariant de la classe que (m!)._values n'est jamais nul.

---

Je connais les invariants de boucle, mais pour la classe... tu peux m'en dire plus ?

Citation:

---

Envoyé par Luc Hermitte

- J'aurai défini un fonction offset, cela sert toujours.

---

C'est-à-dire ? Une méthode qui décale les éléments d'une matrice ? Une translation au sens 3D du terme ?

Citation:

---

Envoyé par Luc Hermitte

Plein de fonctions (add, mult, ..) qui ne devraient pas exister. Si tu les as déjà croisées, c'est la faute à des cours qui prennent trop leur temps pour présenter la surcharge des opérateurs.

---

Des cours qui ne prennent PAS assez leurs temps sur la surcharge d'opérateurs (+,*) tu veux dire ?
add, mult c'est plutôt mon coté Java qui parle. Mais j'ai pensé au deux : ceux qui veulent du concis m*m2+m3^3 ou du m3.pow(3).add(m.mult(m2)). C'est selon.

Citation:

---

Envoyé par Iradrille

Avec ça Matrix<float> * Matrix<double> donne un résultat Matrix<float>, alors que Matrix<double> * Matrix<float> donnera une Matrix<double>.

---

Comment je procède pour prendre le "plus grand des deux types"

Citation:

---

Envoyé par Iradrille

template <class T, unsigned W, unsigned H>
class SmallMatrix { };

template <class T>
class LargeMatrix { };

---

Comment je peux ne pas tout copier coller quasiment ?
Car finalement j'ai que la gestion d'une variable membre qui diffère.
J'ai pensé à l'héritage d'une Small et LargeMatrix à partir d'un Matrix qui contient row/col.

a- AMA, Pas besoin d'itérateurs pour des matrices. matrix(i,j) devraient renvoyer une référence constante ou non, et de là, on peut obtenir un pointeur, c'est à dire un itérateur trivial sur des lignes.

b- On ne fait pas de retours const (depuis le C++11, c'est devenu une mauvaise idée -- car elle peut inhiber la sémantique de déplacement qui est automagiquement fournie par le compilo si une copy-elision n'est pas possible)
Par contre, il faudrait implémenter constructeur et opérateur de déplacement.

c- Il n'y a vraiment aucun intérêt à faire ce test anti-auto affectation. Il ne doit son existence qu'aux codes qui procédaient à l'envers (1- désallocation, 2- allocation, 3- copie). Sauf que l'allocation peut échouer, et donc pour écrire du code correct, il devient nécessaire : il faut mettre l'étape d'allocation en premier et là ... plus besoin de protéger contre l'auto-affectation. La protection devient une pessimisation vu la rareté de la chose dans du code usuel.

d- L'avantage de l'assertion : investiguer les erreurs. Les exceptions ne sont pas adaptées à cela. Et de plus, on va plutôt les utiliser pour faire de la programmation défensive, et donc les garder en Release, or tu ne les lèves qu'en Debug.
Cf un brouillon d'un billet que je suis en train de rédiger : https://github.com/LucHermitte/LucHe...tions.markdown (licence CC-by-NC-SA) [NB: le nom final du fichier risque de changer une fois que je publierai ; le billet 1 parle plus en détail de la programmation défensive ; et BTW pour ceux qui s'embêtent, je suis preneur de commentaires sur la série de billets sur la PpC]

e- par offset(), j'entends la fonction internet qui calcule l'offset qui donne l'index de l'élément dans la structure interne à partir des coordonnées x et y. Cf l'exemple de classe matrice.

f- Non franchement, oublie add(), mult(), ... On n'est pas en Java mais en C++ :p

g- pour l'histoire de choisir le bon type dans Matrix<double>+Matrix<float>, l'expression clé est "promotion de type". Il y avait de telles choses dans boost. Elles ont peut-être fini à l'identique dans le C++11, mais toujours est-il que ce dernier offre d'autres moyen avec decl et -> dans les signatures de fonctions. Je n'ai plus les détails en tête. Avec une petite recherche rapide sur google, SO, ou wikipédia, cela se retrouve.

Citation:

---

Envoyé par Luc Hermitte

a- AMA, Pas besoin d'itérateurs pour des matrices. matrix(i,j) devraient renvoyer une référence constante ou non, et de là, on peut obtenir un pointeur, c'est à dire un itérateur trivial sur des lignes.

---

Pas trop d’accord avec ce point : tu exposes un détail de ton implémentation (le fait qu’elle soit contigue selon les lignes). Je sais que pour des raisons de perf, l’utilisateur de la classe devra bien avoir cette info, mais je préfère autant lui donner au travers d’un itérateur typé (de la même manière que std::vector).

Citation:

---

Cf un brouillon d'un billet que je suis en train de rédiger : https://github.com/LucHermitte/LucHe...tions.markdown (licence CC-by-NC-SA) [NB: le nom final du fichier risque de changer une fois que je publierai ; le billet 1 parle plus en détail de la programmation défensive ; et BTW pour ceux qui s'embêtent, je suis preneur de commentaires sur la série de billets sur la PpC]

---

Je jette un œil, mais tu connais le sujet au moins aussi bien que moi.

Citation:

---

Envoyé par Spleeen

Comment je procède pour prendre le "plus grand des deux types"

---

Comme le dit @Luc Hermitte, en C++11 c'est un job pour decltype

Code:

---

1 2 3 4 5 6 7 8 9 10

template <class T> class Matrix { };   template <class T, class U> Matrix<typename decltype(T() + U())> operator+(Matrix<T> const& lhs, Matrix<U> const& rhs) { typedef decltype(T() + U()) elt_type; typedef Matrix<elt_type> mat_type;   return mat_type(); }

---

Sans C++11, c'est beaucoup plus chiant, mais ça reste un problème relativement courant donc ça doit être présent dans boost.

Citation:

---

Envoyé par Spleeen

Comment je peux ne pas tout copier coller quasiment ?
Car finalement j'ai que la gestion d'une variable membre qui diffère.
J'ai pensé à l'héritage d'une Small et LargeMatrix à partir d'un Matrix qui contient row/col.

---

J'y serai aller à grand coup de #define je pense (pas de meilleure idée à l'esprit)

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25

#define MATRIX_TRANSPOSE_IMPL(type, ...) \ type transpose() const { \ type transposeMat ( __VA_ARGS__ ); \ for(int i=0 ; i < _cols ; ++i) { \ for(int j=0 ; j < _rows ; ++j) { \ transposeMat.set(i,j,at(j,i)); \ } \ } \ return transposeMat; \ } \ /**/   template <class T, unsigned W, unsigned H> class SmallMatrix { MATRIX_TRANSPOSE_IMPL(SmallMatrix); // ... };   template <class T> class LargeMatrix { MATRIX_TRANSPOSE_IMPL(LargeMatrix, _cols, _rows ); // ... };   #undef MATRIX_TRANSPOSE_IMPL

---

:aie:

edit :

Citation:

---

Envoyé par Luc Hermitte

https://github.com/LucHermitte/LucHe...tions.markdown

---

Petit détail :

Code:

---

1 2 3 4 5

double sqrt(double n) { assert(n >=0); const double result = std::sqrt(n); assert(std::abs(result*result - n) < std::numeric_limits<double>::epsilon() * 100); }

---

Il manque un return. ;)

Citation:

---

J'y serai aller à grand coup de #define je pense (pas de meilleure idée à l'esprit)

---

Ça me parait être ultra violent. Le debugger ne va rien voir. En fait je sais pas si c'est une blague xD
Je ne comprends pas encore comment fonctionne le decltype à dire vrai et je trouve peu d'exemple simple l'utilisant.
Je continue mes recherches

[Merci Iradrille]
Pour le transpose, pourquoi un #define ? Les templates sont typiquement faits pour ce genre de choses.

#define est la solution de la facilité. Il y à probablement moyen de faire mieux mais je vois juste pas vraiment comment.

On est deux ^^'
Mise à part faire de l'héritage et tout copier-coller, juste pour une variable gérée différemment...