Mon programme test POO

Version imprimable

Bonjour à tous, ;)

En ce moment, je suis un cour pour apprendre à programmer en C++ et j'ai eu envie de tester mes connaissances en POO en codant un programme sur les fraction (addition, multiplication, comparaison) en utilisant des opérateurs et des classes. En fait je veux juste avoir votre avis sur mon programme. Cependant le code source n'est pas très lisible et non optimisé donc soyez indulgent :ccool:

Voila le main.cpp :

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
#include <iostream>
#include <ostream>
#include "prototype.h"
 
using namespace std;
 
int main()
{
Fraction a(4), b(2,7); // Par defaut "a" aura comme denominateur 1
Fraction c, d;
 
 
d = a * b;
c = a + b;
 
 
d.simplifie();
c.simplifie(); // Simplification des resultats avant affichage sur la console
 
 
cout << a << " + " << b << " = " << c << endl;
 
cout << a << " * " << b << " = " << d << endl; // Affichage du calcul
 
    if(a > b) // Ici je ne vais tout de meme pas vous faire un dessin ^^
        cout << "a est plus grand que b." << endl;
    else if(a==b)
        cout << "a est egal a b." << endl;
    else
        cout << "a est plus petit que b." << endl;
 
    return 0;
}

Et pour le fonction.cpp :

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
#include <ostream>
#include "prototype.h"
 
using namespace std;
 
Fraction::Fraction(int numerateur, int denominateur) : m_numerateur(numerateur), m_denominateur(denominateur)
{
 
}
 
void Fraction::afficher(ostream& flux) const
{
if(m_denominateur != 1)
{
    flux << m_numerateur << "/" << m_denominateur;
}
else
{
    flux << m_numerateur;
}
}
 
 
Fraction operator+(Fraction const& a, Fraction const& b)
{
   Fraction resultat;
 
   resultat = a+b;
 
    return resultat;
}
 
  Fraction& Fraction::operator+(Fraction const& b)
  {
   Fraction resultat;
 
int copie;
int copie2;
 
 
resultat.m_denominateur = m_denominateur * b.m_denominateur;
 
copie = m_denominateur * b.m_numerateur;
 
copie2 = m_numerateur * b.m_denominateur;
 
resultat.m_numerateur = copie + copie2;
 
      return resultat;
  }
 
ostream& operator<<(ostream &flux, Fraction const& fraction)
{
    fraction.afficher(flux);
 
    return flux;
}
 
Fraction& Fraction::operator*(Fraction const& b)
{
   Fraction resultat;
 
   resultat.m_numerateur = m_numerateur * b.m_numerateur;
   resultat.m_denominateur = m_denominateur * b.m_denominateur;
 
    return resultat;
}
 
bool operator>(Fraction const& a, Fraction const& b)
{
bool tf;
 
tf = a.estMoin(b);
 
    return tf;
}
 
 
bool operator==(Fraction const& a, Fraction const& b)
{
   bool tf;
 
   tf = a.estEgal(b);
 
   return tf;
}
 
 
bool Fraction::estEgal(Fraction const& b) const
{
 int test;
 int test2;
 
    test = m_numerateur / m_denominateur;
 
    test2 = b.m_numerateur / b.m_denominateur;
 
    if(test == test2)
    {
        return true;
    }
    else
    {
    return false;
    }
 
}
bool Fraction::estMoin(Fraction const& b) const
{
   int test;
   int test2;
 
   test = m_numerateur / m_denominateur;
 
    test2 = b.m_numerateur / b.m_denominateur;
 
    if(test > test2)
    {
    return true;
    }
    else
    {
    return false;
    }
}
 
 // Aide de Matheo pour la simplification
 
int pgcd(int a, int b)
{
    while (b != 0)
    {
        const int t = b;
        b = a%b;
        a=t;
    }
    return a;
}
 
void Fraction::simplifie()
{
    int nombre=pgcd(m_numerateur, m_denominateur);
 
    m_numerateur /= nombre;
    m_denominateur /= nombre;
}

Enfin le prototype.h :

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
#ifndef PROTOTYPE_H_INCLUDED
#define PROTOTYPE_H_INCLUDED
 
#include <ostream>
 
class Fraction
{
  public:
 
  void afficher(std::ostream &flux) const;
  Fraction(int numerateur = 0, int denominateur = 1);
  Fraction& operator+(Fraction const& b);
  Fraction& operator*(Fraction const& b);
  bool estEgal(Fraction const& b) const;
bool estMoin(Fraction const& b) const;
void simplifie();
 
  private:
 
    int m_numerateur;
    int m_denominateur;
};
 
Fraction operator+(Fraction const& a, Fraction const& b);
std::ostream& operator<<(std::ostream &flux, Fraction const& fraction);
Fraction operator*(Fraction const& a, Fraction const& b);
bool operator>(Fraction const& a, Fraction const& b);
bool operator==(Fraction const& a, Fraction const& b);
int pgcd(int a, int b);
 
 
#endif // PROTOTYPE_H_INCLUDED

Voila :)

ps : Désolé pour les fautes d'orthographes. Mon IDE : Code Block :mrgreen:

Salut,

L'indentation du code n'est pas propre.

On va dire que ce n'est pas si mal pour un débutant mais j'ai les remarques suivantes :

Ta méthode "simplifier" est carrément inutile, pourquoi ne pas renvoyer à chaque fois des fractions déjà simplifiées ?
Tu utilises une méthode "estEgal" et "estMoins" alors que tu surcharges les opérateurs "==" et "<", tu n'as pas bien compris leur but j'ai l'impression. Idem pour afficher.
Les noms de fichier sont nazes, pourquoi ne pas utiliser "fraction.cpp" et "fraction.hpp" ?
Il faudrait que tu surcharges tout les opérateurs et prévoir les cas ou tu veux ajouter un entier à une fraction (et non une fraction à une fraction).
Pourquoi est-ce que tu retournes des références ?
Tu ne gères pas le cas où le dénominateur est égal à 0.

A part ça, tu pourrais simplifier ton code, par exemple pour l'opérateur "+" (en gérant le dénominateur à 0 et en retournant une fraction simplifiée) :
Code:

1 2 3 4 5 6 7 8 9 10 Fraction Fraction::operator+(Fraction const& x) { if(den == 0) return x; else if(x.den == 0) return *this; long int p = ppcm(x.den, den); return Fraction(p/den*num + p/x.den*x.num, p); }

18/03/2012, 18h34
Pirodactil

Je te remercie pour tes remarques constructives :ccool: . Je reconnais ne pas avoir passer beaucoup de temps sur mon code mais mon but n'était pas vraiment là. Je voulais juste voir si j'ai bien compris quand, pourquoi et comment me servir des opérateurs, des classes et des constructeurs. ;)

Mais dans un sens tu as raison, je dois impérativement m'occuper de tous les cas : par exemple le dénominateur à 0. ;) Et en se qui concerne les opérateurs, on m'a appris à m'en servir comme cela donc si tu sais faire autrement dis-moi comment. 8O Toutes les remarques sont les bien venues :mrgreen:
18/03/2012, 18h49
Ekleog
Citation:

Envoyé par Pirodactil

Je te remercie pour tes remarques constructives :ccool: . Je reconnais ne pas avoir passer beaucoup de temps sur mon code mais mon but n'était pas vraiment là. Je voulais juste voir si j'ai bien compris quand, pourquoi et comment me servir des opérateurs, des classes et des constructeurs. ;)

Mais dans un sens tu as raison, je dois impérativement m'occuper de tous les cas : par exemple le dénominateur à 0. ;) Et en se qui concerne les opérateurs, on m'a appris à m'en servir comme cela donc si tu sais faire autrement dis-moi comment. 8O Toutes les remarques sont les bien venues :mrgreen:

Et pourtant ... Pour citer mon voisin du dessus, il y a des remarques correspondant précisément à ta volonté : "voir si j'ai bien compris quand, pourquoi et comment me servir des opérateurs, des classes et des constructeurs."
J'ai retiré les points ne correspondant pas exactement à ta description, mais pourtant ils devraient être pris en considération, évidemment, même si le code n'a pas pour but d'être efficace/propre/autre, mais simplement parce qu'il faut s'entrainer pour bien programmer !
Citation:
Envoyé par Trademark

[...]

[...]
Tu utilises une méthode "estEgal" et "estMoins" alors que tu surcharges les opérateurs "==" et "<", tu n'as pas bien compris leur but j'ai l'impression. Idem pour afficher.
[...]
Il faudrait que tu surcharges tout les opérateurs et prévoir les cas ou tu veux ajouter un entier à une fraction (et non une fraction à une fraction).
Pourquoi est-ce que tu retournes des références ?
[...]

[...]
Au passage, Trademark, il me semble que tu as fait une erreur pour la priorité des opérateurs dans ton bout de code : return Fraction(p/den*num + p/x.den*x.num, p); calcule ppcm(den, x.den) / den ; qui est égal à 0 ou 1.
Il me semble qu'il faudrait plutôt utiliser (de tête, non testé) : return Fraction(p * num / den + p * x.num / x.den, p); (joies de l'arithmétique entière qui tronque au milieu du calcul).
18/03/2012, 20h34
Pirodactil

Je tiendrais compte de vos conseils à l'avenir.

Merci à vous deux. :ccool:

Salut,
En + des remarques pertinentes de Trademark, j'ajouterais :
Code:

1 2 3 4 5 6 7 8 9 10 class Fraction { // snip Fraction& operator+(Fraction const& b); // (1) Fraction& operator*(Fraction const& b); // (1) // snip }; Fraction operator+(Fraction const& a, Fraction const& b); // (2) Fraction operator*(Fraction const& a, Fraction const& b); // (2)
Là tu as deux erreurs. La première est que la définition de l'opérateur dans la classe (1) devrait être constante :
Code:

1 2 3 4 5 6 7 class Fraction { // snip Fraction& operator+(Fraction const& b) const; // (1) Fraction& operator*(Fraction const& b) const; // (1) // snip };
Ce qui révèle ta seconde erreur : les opérateurs sont définis à la fois à l'intérieur de la classe et à l'extérieur. Il faut choisir ! La définition à l'extérieur de la classe avec en se basant sur la version += est une approche classique :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 class Fraction { // snip Fraction const & operator+=(Fraction const& b) const; // (1) Fraction const & operator*=(Fraction const& b) const; // (1) // snip }; Fraction const operator+(Fraction const& a, Fraction const& b) { Fraction temp(a); return temp+=b; } Fraction const operator*(Fraction const& a, Fraction const& b) { Fraction temp(a); return temp*=b; }
Dans ton code existant, tes opérateurs à l'intérieur de la classe renvoi une référence sur une variable temporaire de la fonction :
Code:

1 2 3 4 5 6 Fraction& Fraction::operator+(Fraction const& b) { Fraction resultat; // snip return resultat; }
=> plantage garanti à un moment ou un autre !

Ton constructeur, à partir du moment où le second argument peut être omis, devrait être explicit :
Code:

1 2 3 4 class Fraction { public: explicit Fraction(int numerateur = 0, int denominateur = 1);
Personnellement, je n'aime pas mettre l'affichage dans les classes. Est-ce vraiment de la responsabilité de Fraction de gérer son affichage ? Qui si celui-ci doit changer ou être différent selon le fichier (xml, text, csv, etc.) ? Je préfère donner une amitié et mettre cet affichage complètement à l'extérieur :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 class Fraction { friend std::ostream& operator<<(std::ostream &flux, Fraction const& fraction); //snip}; ostream& operator<<(ostream &flux, Fraction const& fraction) { if(fraction.m_denominateur != 1) { flux << fraction.m_numerateur << "/" << fraction.m_denominateur; } else { flux << fraction.m_numerateur; } return flux; }
Tes fonctions EstEgal et EstMoin (au passage estMoins, tu ne dois pas connaître de marseillais sinon tu saurais que té ! c'est mouaingsssssssss) sont fausses. L'arithmétique entière sur laquelle tu t'appuie fait que :
Code:

1 2 3 4 a = Fraction(1,3); b = Fraction(2,3); if(a==b) cout << "a est egal a b." << endl; // on passe ici
et
Code:

1 2 3 4 5 6 7 a = Fraction(1,3); b = Fraction(2,3); if(b > a) // Faux cout << "a est plus grand que b." << endl; if(a > b) // Faux cout << "a est plus grand que b." << endl;
Pour le test d'égalité, le mieux est de simplifier et comparer strictement numérateur et dénominateur. Pour les opérateurs de comparaison, c'est un peu plus subtil.