Code de gestion de matrices, merci de le critiquer

Signaler
Messages postés
5
Date d'inscription
dimanche 25 février 2007
Statut
Membre
Dernière intervention
11 février 2012
-
Messages postés
5
Date d'inscription
dimanche 25 février 2007
Statut
Membre
Dernière intervention
11 février 2012
-
#ifndef MATRIX_H
#define MATRIX_H

#include
#include <vector>

template<class T>
class Matrix
{
private:
int _nbLignes, _nbColonnes;
std::vector<std::vector<T>> _v;
void setElement(int l, int c, T v);
Matrix():_nbLignes(0),_nbColonnes(0){};

public:
Matrix(int, int);
Matrix(const Matrix&);

void operator+=(const Matrix&);
void operator-=(const Matrix&);

Matrix operator+(const Matrix&) const;
Matrix operator-(const Matrix&) const;

T operator()(int, int) const;

int getNbLignes() const{return _nbLignes;}
int getNbColonnes() const{return _nbColonnes;}

//initialise
void fill(T value);

int set(int l, int c, T v);

bool operator==(const Matrix&) const;

};

template<class T>
std::ostream& operator << (std::ostream& out, const Matrix<T>& m)
{
for(int i=0; i<m.getNbLignes(); i++)
{
out << "|" ;

for(int j=0; j<m.getNbColonnes(); j++)
out << " " << m(i,j) << " " ;

out << "|" << std::endl;
}
return out;
}

template<class T>
Matrix<T>::Matrix(int nbLignes, int nbColonnes):_nbLignes(nbLignes), _nbColonnes(nbColonnes)
{
_v.resize(nbLignes);
for(int i=0; i<nbLignes; i++)
_v[i].resize(nbColonnes);
}

template<class T>
Matrix<T>::Matrix(const Matrix<T>& m)
{
_nbLignes = m._nbLignes;
_nbColonnes = m._nbColonnes;

_v.resize(_nbLignes);
for(int i=0; i<_nbLignes; i++)
_v[i].resize(_nbColonnes);

for(int i=0; i<_nbLignes; i++)
for(int j=0; j<_nbColonnes; j++)
setElement(i,j,m(i,j));
}

template<class T>
void Matrix<T>::operator+=(const Matrix<T>& m)
{
if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
{
std::cout << "addition impossible !!" << std::endl;
return ;
}

for(int i=0; i<_nbLignes; i++)
for(int j=0; j<_nbColonnes; j++)
this->setElement(i,j,m(i,j)+_v[i][j]);
}

template<class T>
void Matrix<T>::operator-=(const Matrix<T>& m)
{
if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
{
std::cout << "soustraction impossible !!" << std::endl;
return ;
}

for(int i=0; i<_nbLignes; i++)
for(int j=0; j<_nbColonnes; j++)
this->setElement(i,j,_v[i][j]-m(i,j));
}

template<class T>
Matrix<T> Matrix<T>::operator+(const Matrix<T>& m) const
{
if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
{
std::cout << "addition impossible !!" << std::endl;
return Matrix<T>();
}

Matrix result(*this);
result+=m;
return result;
}

template<class T>
Matrix<T> Matrix<T>::operator-(const Matrix<T>& m) const
{
if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
{
std::cout << "soustraction impossible !!" << std::endl;
return Matrix<T>();
}

Matrix result(*this);
result-=m;
return result;
}

template<class T>
T Matrix<T>::operator()(int row, int col) const
{
return (this->_v[row])[col];
}

//initialiser
template<class T>
void Matrix<T>::fill(T value)
{
for(int i=0; i<_nbLignes; i++)
for(int j=0; j<_nbColonnes; j++)
this->setElement(i,j,value);
}

template<class T>
void Matrix<T>::setElement(int row, int col, T value)
{
(_v[row])[col] = value;
}

template<class T>
int Matrix<T>::set(int row, int col, T value)
{
if(row>0 && col>0 && row<=_nbLignes && col<_nbColonnes)
(_v[row-1])[col-1] = value;
else
return -1;

return 0;
}

template<class T>
bool Matrix<T>::operator==(const Matrix<T>& m) const
{
if(_nbLignes!= m._nbLignes || _nbColonnes!=m._nbColonnes)
return false;

for(int i=0; i<_nbLignes; i++)
for(int j=0; j<_nbColonnes; j++)
if (_v[i][j]!= m(i,j)) return false;

return true;
}

#endif

6 réponses

Messages postés
305
Date d'inscription
jeudi 29 avril 2004
Statut
Membre
Dernière intervention
18 janvier 2012

Bonjour
Bien bien bien ....
jolie classe ...
Mais c'est quoi la question ?
C'est vraiment lourd à force ce genre de post...
Messages postés
3819
Date d'inscription
dimanche 12 décembre 2004
Statut
Modérateur
Dernière intervention
28 septembre 2020
113
Bonjour.

La question est dans le titre, mais il aurait été bien de la rappeler dans le sujet. Pense aussi à utiliser la balise de code quand tu en postes, sinon c'est un peu illisible.
Sépare aussi chacun des fichiers dans une balise de code différente, en précisant le nom du fichier.
Une fois cette opération faite, on pourra jeter un coup d'oeil sur ton travail.

Voici un exemple de question bien posée: http://www.cppfrance.com/forum/sujet-DECLARER-OBJETS-DANS-ATTRIBUTS-CLASSE-DEFINISSANT-OBJET-DECLARE_1540447.aspx

________________________________________________________________________
Historique de mes créations, et quelques articles:
[ http://0217021.free.fr/portfolio http://0217021.free.fr/portfolio]
Merci d'utiliser Réponse acceptée si un post répond à votre question
Messages postés
3819
Date d'inscription
dimanche 12 décembre 2004
Statut
Modérateur
Dernière intervention
28 septembre 2020
113
@imed07: Je viens de supprimer ton message. Merci de faire attention à l'indentation. La couleur c'est bien, mais ça ne suffit pas. L'indentation est importante aussi.
Reposte avec ce critère supplémentaire.

________________________________________________________________________
Historique de mes créations, et quelques articles:
[ http://0217021.free.fr/portfolio http://0217021.free.fr/portfolio]
Merci d'utiliser Réponse acceptée si un post répond à votre question
Messages postés
5
Date d'inscription
dimanche 25 février 2007
Statut
Membre
Dernière intervention
11 février 2012

#ifndef MATRIX_H
#define MATRIX_H

#include 
#include <vector>

template<class T>
class Matrix
{
private:
   int _nbLignes, _nbColonnes;
   std::vector<std::vector<T>> _v;
   void setElement(int l, int c, T v);
   Matrix():_nbLignes(0),_nbColonnes(0){};

public:
   Matrix(int, int);
   Matrix(const Matrix&);

   void operator+=(const Matrix&);
   void operator-=(const Matrix&);

   Matrix operator+(const Matrix&) const;
   Matrix operator-(const Matrix&) const;

   T operator()(int, int) const;

   int getNbLignes() const{return _nbLignes;}
   int getNbColonnes() const{return _nbColonnes;}

   //initialise
   void fill(T value);

   int set(int l, int c, T v);

   bool operator==(const Matrix&) const;

};

template<class T>
std::ostream& operator << (std::ostream& out, const Matrix<T>& m)
{
  for(int i=0; i<m.getNbLignes(); i++)
  {
     out << "|" ;

     for(int j=0; j<m.getNbColonnes(); j++)
        out << " " << m(i,j) << " " ;

     out << "|" << std::endl;
  }
return out;
}

template<class T>
Matrix<T>::Matrix(int nbLignes, int nbColonnes):_nbLignes(nbLignes), _nbColonnes(nbColonnes)
{
   _v.resize(nbLignes);
   for(int i=0; i<nbLignes; i++)
      _v[i].resize(nbColonnes);
}

template<class T>
Matrix<T>::Matrix(const Matrix<T>& m)
{
   _nbLignes = m._nbLignes;
   _nbColonnes = m._nbColonnes;

   _v.resize(_nbLignes);
   for(int i=0; i<_nbLignes; i++)
      _v[i].resize(_nbColonnes);

   for(int i=0; i<_nbLignes; i++)
      for(int j=0; j<_nbColonnes; j++)
         setElement(i,j,m(i,j));
}

template<class T>
void Matrix<T>::operator+=(const Matrix<T>& m)
{
   if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
   {
       std::cout << "addition impossible !!" << std::endl;
       return ;
   }

for(int i=0; i<_nbLignes; i++)
   for(int j=0; j<_nbColonnes; j++)
      this->setElement(i,j,m(i,j)+_v[i][j]);
}

template<class T>
void Matrix<T>::operator-=(const Matrix<T>& m)
{
   if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
   {
       std::cout << "soustraction impossible !!" << std::endl;
       return ;
   }

for(int i=0; i<_nbLignes; i++)
   for(int j=0; j<_nbColonnes; j++)
      this->setElement(i,j,_v[i][j]-m(i,j));
}

template<class T>
Matrix<T> Matrix<T>::operator+(const Matrix<T>& m) const
{
   if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
   {
      std::cout << "addition impossible !!" << std::endl;
      return Matrix<T>();
   }

   Matrix result(*this);
   result+=m;
   return result;
}

template<class T>
Matrix<T> Matrix<T>::operator-(const Matrix<T>& m) const
{
   if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
   {
      std::cout << "soustraction impossible !!" << std::endl;
      return Matrix<T>();
   }

   Matrix result(*this);
   result-=m;
   return result;
}

template<class T>
T Matrix<T>::operator()(int row, int col) const
{
   return (this->_v[row])[col];
}

//initialiser
template<class T>
void Matrix<T>::fill(T value)
{
   for(int i=0; i<_nbLignes; i++)
      for(int j=0; j<_nbColonnes; j++)
         this->setElement(i,j,value);
}

template<class T>
void Matrix<T>::setElement(int row, int col, T value)
{
   (_v[row])[col] = value;
}

template<class T>
int Matrix<T>::set(int row, int col, T value)
{
   if(row>0 && col>0 && row<=_nbLignes && col<_nbColonnes)
      (_v[row-1])[col-1] = value;
   else
      return -1;

   return 0;
}

template<class T>
bool Matrix<T>::operator==(const Matrix<T>& m) const
{
   if(_nbLignes!= m._nbLignes || _nbColonnes!=m._nbColonnes)
      return false;

   for(int i=0; i<_nbLignes; i++)
      for(int j=0; j<_nbColonnes; j++)
         if (_v[i][j]!= m(i,j)) return false;

   return true;
}

#endif 
Messages postés
3819
Date d'inscription
dimanche 12 décembre 2004
Statut
Modérateur
Dernière intervention
28 septembre 2020
113
C'est déjà beaucoup plus lisible.

Voici quelques critiques:
- Matrix(int, int); => Mieux vaut laisser le nom des arguments dans la définition. C'est beaucoup plus clair. => Matrix(int nbLignes, int nbColonnes)
- On sépare le code de la définition. La définition va dans .hh et le code templaté dans un .hxx (le .hxx est inclus à la fin du .hh, juste avant le #endif).
- Utilise toujours la liste d'initialisation. Tu l'as bien fait pour le premier constructeur, tu peux aussi le faire pour le deuxième.
template<class T>
Matrix<T>::Matrix(const Matrix<T>& m)
{
   _nbLignes = m._nbLignes;
   _nbColonnes = m._nbColonnes;

devient:
template<class T>
Matrix<T>::Matrix(const Matrix<T>& m)
 : _nbLignes(m._nbLignes), _nbColonnes (m._nbColonnes)
{

- Tu peux directement assigné des valeurs à ton tableau, au lieu de refaire une boucle:
   _v.resize(_nbLignes);
   for(int i=0; i<_nbLignes; i++)
      _v[i].resize(_nbColonnes);

   for(int i=0; i<_nbLignes; i++)
      for(int j=0; j<_nbColonnes; j++)
         setElement(i,j,m(i,j));

Devient:
   _v.resize(_nbLignes);
   for(int i = 0; i < _nbLignes; ++i)
  {
      _v[i].resize(_nbColonnes);
      for(int j = 0; j < _nbColonnes; ++j)
         setElement(i,j,m(i,j));
  }

- Quand on est face à une erreur, on n'écrit pas un message mais on lève plutôt une exception:
   if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
   {
       std::cout << "addition impossible !!" << std::endl;
       return ;
   }

devient:
   if ((_nbLignes != m._nbLignes)||(_nbColonnes != m._nbColonnes))
      throw MatrixException("addition impossible !!");

(Il faut que tu codes une classe MatrixException qui hérite de std::exception)

- Ici:
template<class T>
T Matrix<T>::operator()(int row, int col) const
{
   return (this->_v[row])[col];
}

Tu peux directement écrire:
template<class T>
T Matrix<T>::operator()(int row, int col) const
{
   return _v[row][col];
}

- Idem pour:
(_v[row])[col] = value;

qui peut directement s'écrire:
_v[row][col] = value;

- Ici:
if(row>0 && col>0 && row<=_nbLignes && col<_nbColonnes)
      (_v[row-1])[col-1] = value;
   else
      return -1;

   return 0;

Tu peux lever un exception:
if( row < 0 || col < 0 || row > _nbLignes || col > _nbColonnes)
  throw MatrixException("Index out of bound");

_v[row-1][col-1] = value;

- Dans operator==:
if (_v[i][j]!= m(i,j)) return false;

devient:
if (_v[i][j]!= m._v[i][j]) return false;


Au niveau du design:
- Tu force l'utilisateur à considérer que la matrice commence à 1-1 plutôt que 0-0. C'est pas très naturel et est sujet à erreur. A ta place je ferais démarrer à 0-0 comme tous les conteneurs du C et du C++.
- Au lieu de recoder l'operator<< directement, il est généralement préférable de créer une méthode publique qui fait l'affichage et de l'appeler dans l'operator<<. Ex:

template<class T>
void Matrix<T>::print(std::ostream& out) const
{
  for(int i = 0; i<m._nbLignes; ++i)
  {
     out << "|";
     for(int j = 0; j < m._nbColonnes; ++j)
        out << " " << m._v[i][j] << " ";
     out << "|" << std::endl;
  }
}

Puis en dehors de la classe:
template<class T>
std::ostream& operator<<(std::ostream& out, const Matrix<T>& m)
{
  m.print(out);
  return out;
}


________________________________________________________________________
Historique de mes créations, et quelques articles:
[ http://0217021.free.fr/portfolio http://0217021.free.fr/portfolio]
Merci d'utiliser Réponse acceptée si un post répond à votre question
Messages postés
5
Date d'inscription
dimanche 25 février 2007
Statut
Membre
Dernière intervention
11 février 2012

Merci.