pgl10
Messages postés380Date d'inscriptionsamedi 18 décembre 2004StatutMembreDernière intervention29 octobre 202311 20 févr. 2013 à 08:56
Après avoir fait un envoi en urgence pour corriger une erreur dans le calcul d'un angle voici la version définitive qui lui correspond. Cette dernière version ajoute quelques fonctions supplémentaires pour les points, droites et triangles. Je crois que Mageo3d peut convenir dans cette version pour être utilisable assez souvent dans son domaine. Mais, à mon point de vue, c'est maintenant une bibliothèque très facile à modifier ou à compléter si nécessaire et qui peut aider pour en développer d'autres dans d'autres domaines.
pgl10
Messages postés380Date d'inscriptionsamedi 18 décembre 2004StatutMembreDernière intervention29 octobre 202311 19 févr. 2013 à 13:42
Je suis bien désolé. Je me dois de poster sans plus attendre une nouvelle version pour corriger une erreur de signe dans la formule de calcul d'un angle() Je préfère signaler moi-même cette erreur stupide ! C'est dommage, parce que je n'ai ajouté que la projection d'un point sur une droite. J'avais aussi d'autres fonctions à ajouter, ce sera pour une autre fois ! Et si vous souhaitez une modification et/ou un complément, merci de l'indiquer.
pgl10
Messages postés380Date d'inscriptionsamedi 18 décembre 2004StatutMembreDernière intervention29 octobre 202311 18 févr. 2013 à 13:46
Merci à Ccgousset pour ce commentaire sympathique et surtout merci à CptPingu de nous expliquer les bonnes pratiques en programmation C++ Cette deuxième version est entièrement nouvelle dans la manière d'écrire les programmes. J'ai appliqué ici les recommandations de CptPingu pour améliorer la programmation, l'utilisation et la réutilision de Mageo3d. J'en ai aussi profité pour ajouter le produit vectoriel v % w et la création et l'utilisation d'une base quelconque de vecteurs pour l'espace R3.
ccgousset
Messages postés150Date d'inscriptionsamedi 1 août 2009StatutMembreDernière intervention 4 mars 2023 17 févr. 2013 à 16:09
Salut pgl10, Tu nous refais Matlab ca c'est sympa.
Y a plus qu'a greffer un matric 3x en dibits genre
du lecteur ppm et tous les etudiants de 5eme vont
pouvoir envoyer des hommes sur la lune.
Bravo pour ton bibliotheque, je te pique tout ca
une fois de plus. Bonne journée.
pgl10
Messages postés380Date d'inscriptionsamedi 18 décembre 2004StatutMembreDernière intervention29 octobre 202311 16 févr. 2013 à 20:26
Bonjour CptPingu, Merci beaucoup pour tous ces commentaires. Il y en a quelques uns que je connaissais déjà et que j'aurais dû ou pu éviter ou corriger. On n'est pas toujours soi-même son meilleur observateur. Mais il y en a aussi qui vont me permettre d'approfondir ce que connais du C++ C'est pour cela que CodeS-SourceS et cppfrance maintiennent un forum instructif et sympathique. Pour répercuter les modifications qui sont proposées il faut être assez attentif, cela prendra un peu de temps. Merci aussi de montrer aux visiteurs que pour certaines dispositions il y a un choix à faire. Il y a dans Mageo3d des fonctions en double comme : v*w et scal(v,w) c'est fait volontairement pour pouvoir choisir son style d'utilisation dans divers environnement. Par contre il y a sûrement encore des fonctions habituellement usuelles qui manquent à condition qu'elles soient dans le domaine déjà traité. Je suis preneur si on veut bien les signaler. Cordialement, pgl10
cptpingu
Messages postés3837Date d'inscriptiondimanche 12 décembre 2004StatutModérateurDernière intervention28 mars 2023123 16 févr. 2013 à 17:01
Bonjour.
Pas de remarques fonctionnelles, en revanche quelques petits points techniques perfectibles:
- Un nom de classe porte généralement une majuscule
- Un attribut de classe se différencie d'une variable local par un signe distinctif (par exemple en pré-fixant par '_'). mx => _mx;
- Inclure cmath plutôt que math.h
- Un argument de fonction avec un const, a peu d'intérêt si l'argument n'est pas une référence. Par exemple: v3d(const double a) => pourrait être V3d(double a). En effet, l'argument étant déjà une copie, peu de risque d'affecter la "vraie" variable. Néanmoins, ce n'est ni une erreur, ni une inélégance. C'est une question de goût et l'utilisation que tu en fais est tout à fait légitime. Je ne pointe ici que le fait que le "const" est facultatif.
- Dommage que le code soit dans le header :(. Il aurait été préférable de sortir le code. Si tu veux conserver le fait que le code reste dans un header, tout en séparant le code, met l'implémentation dans un .hxx (voir mes sources pour cette technique). Il faudra juste marquer tes méthodes comme "inline" pour que ça passe.
- Oublie de constitude sur les getters: double getX() => double getX() const
- Evite au maximum le mot clé "friend". Son utilité est très rare, et sa présence est souvent le signe d'une mauvaise conception. Les méthodes que tu as en "friend", pourrait être de simple fonciton ou alors de véritables fonctions membres. Dans ton code actuel, l'intégralité des "friend" peut être éviter.
- A ce sujet, un bon design pour créer un operator>> ou un operator<< est le suivant: Créer une méthode d'affichage qui prend le flux en entrée. Appelé cette fonction dans une fonction operator<< qui est externe à la classe. Ex:
class V3d
{
public:
void print(std::ostream& out) const;
}
std::ostream& operator<<(std::ostream& ost, const v3d& v);
- Il y a des copies inutiles. Ex dans les arguments: const v3d pnt => const v3d& pnt
- Bien déclaré les variables au moment de l'utilisation, afin de limiter au maximumu le scope. Ex: v3d axe; axe=dir; => V3d axe = dir; De même, lorsqu'il y a pré-déclaration, il y a amélioration possibles (je pense surtout au " double a, b, c, d, u, v, w, x, y, z, ct, st;" de la ligne 84.
Ex:
double angle(const v3d p, const v3d q, const v3d r) {
double a, b, c, d;a norme(r-q); b norme(p-r); c = norme(q-p);
if(b*c==0.0) return 0.0;
d = (a*a-b*b-c*c)/(2.0*b*c);
return acos(d);
}
Pourrait être:
double angle(const v3d& p, const v3d& q, const v3d& r)
{
const double a = norme(r - q);
const double b = norme(p - r);
const double c = norme(q - p);
if (b * c == 0.0)
return 0.0;
const double d = (a * a - b * b - c * c) / (2.0 * b * c);
return acos(d);
}
- Pas besoin de "else", si tu as un "return".
- Au niveau des commentaires, évite d'en mettre en parallèle du code (càd, à droite ou à gauche d'une ligne de code). Mieux vaut le mettre au dessus d'une ligne. Des commentaires en Doxygen, aurait été agréable. Voir ce lien pour quelques conseils: http://0217021.free.fr/portfolio/axel.berardino/articles/ecrire-de-bons-commentaires - Au niveau du design, si tu sens qu'une utilisation peut poser problème (comme tu l'expliques dans ton commentaire pour la fonction translate), n'hésite pas à le changer. Prenons la fonction membre "translate". Tu retournes "*this" afin d'autoriser le chaînage d'action. (Du type: v.translate(0, 0, 0).translate(0, 0, 0)". C'est légitime. En revanche, si tu as peur d'une utilisation suspecte du type "v + w.translate(0, 0, 0)" alors tu peux partir sur un design du type:
La fonction membre translate retourne non pas *this, mais un nouveau V3d qui est le résultat de la translation de w. Ainsi, "v + w.translate(0, 0, 0)" ne pose plus de souci. On est en revanche obligé de faire: "w = w.translate(0, 0, 0)" au lieu de simplement "w.translate(0, 0, 0)". C'est ici un choix. Les deux ont autant de qualités et de défauts, et sont correctes.
- On peut chaîner les std::cout plus efficacement dans ta démo. Exemple:
std::cout << " Le centre du triangle PQR est \205 : " << centre(p,q,r) << std::endl;
std::cout << " On peut aussi le retrouver avec : " << (p+q+r)/3.0 << std::endl;
std::cout << " La surface du triangle PQR vaut : " << surf(p,q,r) << std::endl;
pourrait être:
std::cout << " Le centre du triangle PQR est \205 : " << centre(p,q,r) << std::endl
<< " On peut aussi le retrouver avec : " << (p+q+r)/3.0 << std::endl
<< " La surface du triangle PQR vaut : " << surf(p,q,r) << std::endl;
- Je remplacerais "copyright pgl10" par "@author pgl10". Il n'y a pas de copyright sur un site de partage, je pense que tu voulais simplement mettre l'auteur.
J'ai émis beaucoup de remarques, mais ça ne remet pas en question la qualité du code présenté.
20 févr. 2013 à 08:56
19 févr. 2013 à 13:42
18 févr. 2013 à 13:46
17 févr. 2013 à 16:09
Y a plus qu'a greffer un matric 3x en dibits genre
du lecteur ppm et tous les etudiants de 5eme vont
pouvoir envoyer des hommes sur la lune.
Bravo pour ton bibliotheque, je te pique tout ca
une fois de plus. Bonne journée.
16 févr. 2013 à 20:26
16 févr. 2013 à 17:01
Pas de remarques fonctionnelles, en revanche quelques petits points techniques perfectibles:
- Un nom de classe porte généralement une majuscule
- Un attribut de classe se différencie d'une variable local par un signe distinctif (par exemple en pré-fixant par '_'). mx => _mx;
- Inclure cmath plutôt que math.h
- Un argument de fonction avec un const, a peu d'intérêt si l'argument n'est pas une référence. Par exemple: v3d(const double a) => pourrait être V3d(double a). En effet, l'argument étant déjà une copie, peu de risque d'affecter la "vraie" variable. Néanmoins, ce n'est ni une erreur, ni une inélégance. C'est une question de goût et l'utilisation que tu en fais est tout à fait légitime. Je ne pointe ici que le fait que le "const" est facultatif.
- Dommage que le code soit dans le header :(. Il aurait été préférable de sortir le code. Si tu veux conserver le fait que le code reste dans un header, tout en séparant le code, met l'implémentation dans un .hxx (voir mes sources pour cette technique). Il faudra juste marquer tes méthodes comme "inline" pour que ça passe.
- Oublie de constitude sur les getters: double getX() => double getX() const
- Evite au maximum le mot clé "friend". Son utilité est très rare, et sa présence est souvent le signe d'une mauvaise conception. Les méthodes que tu as en "friend", pourrait être de simple fonciton ou alors de véritables fonctions membres. Dans ton code actuel, l'intégralité des "friend" peut être éviter.
- A ce sujet, un bon design pour créer un operator>> ou un operator<< est le suivant: Créer une méthode d'affichage qui prend le flux en entrée. Appelé cette fonction dans une fonction operator<< qui est externe à la classe. Ex:
class V3d
{
public:
void print(std::ostream& out) const;
}
std::ostream& operator<<(std::ostream& ost, const v3d& v);
void
V3d::print(std::ostream& out) const
{
out << "[" << v._mx << ", " << v._my << ", " << v._mz << "]";
}
std::ostream& operator<<(std::ostream& ost, const V3d& v)
{
v.print(ost);
return ost;
};
- Il y a des copies inutiles. Ex dans les arguments: const v3d pnt => const v3d& pnt
- Bien déclaré les variables au moment de l'utilisation, afin de limiter au maximumu le scope. Ex: v3d axe; axe=dir; => V3d axe = dir; De même, lorsqu'il y a pré-déclaration, il y a amélioration possibles (je pense surtout au " double a, b, c, d, u, v, w, x, y, z, ct, st;" de la ligne 84.
Ex:
double angle(const v3d p, const v3d q, const v3d r) {
double a, b, c, d;a norme(r-q); b norme(p-r); c = norme(q-p);
if(b*c==0.0) return 0.0;
d = (a*a-b*b-c*c)/(2.0*b*c);
return acos(d);
}
Pourrait être:
double angle(const v3d& p, const v3d& q, const v3d& r)
{
const double a = norme(r - q);
const double b = norme(p - r);
const double c = norme(q - p);
if (b * c == 0.0)
return 0.0;
const double d = (a * a - b * b - c * c) / (2.0 * b * c);
return acos(d);
}
- Pas besoin de "else", si tu as un "return".
- Au niveau des commentaires, évite d'en mettre en parallèle du code (càd, à droite ou à gauche d'une ligne de code). Mieux vaut le mettre au dessus d'une ligne. Des commentaires en Doxygen, aurait été agréable. Voir ce lien pour quelques conseils: http://0217021.free.fr/portfolio/axel.berardino/articles/ecrire-de-bons-commentaires
- Au niveau du design, si tu sens qu'une utilisation peut poser problème (comme tu l'expliques dans ton commentaire pour la fonction translate), n'hésite pas à le changer. Prenons la fonction membre "translate". Tu retournes "*this" afin d'autoriser le chaînage d'action. (Du type: v.translate(0, 0, 0).translate(0, 0, 0)". C'est légitime. En revanche, si tu as peur d'une utilisation suspecte du type "v + w.translate(0, 0, 0)" alors tu peux partir sur un design du type:
La fonction membre translate retourne non pas *this, mais un nouveau V3d qui est le résultat de la translation de w. Ainsi, "v + w.translate(0, 0, 0)" ne pose plus de souci. On est en revanche obligé de faire: "w = w.translate(0, 0, 0)" au lieu de simplement "w.translate(0, 0, 0)". C'est ici un choix. Les deux ont autant de qualités et de défauts, et sont correctes.
- On peut chaîner les std::cout plus efficacement dans ta démo. Exemple:
std::cout << " Le centre du triangle PQR est \205 : " << centre(p,q,r) << std::endl;
std::cout << " On peut aussi le retrouver avec : " << (p+q+r)/3.0 << std::endl;
std::cout << " La surface du triangle PQR vaut : " << surf(p,q,r) << std::endl;
pourrait être:
std::cout << " Le centre du triangle PQR est \205 : " << centre(p,q,r) << std::endl
<< " On peut aussi le retrouver avec : " << (p+q+r)/3.0 << std::endl
<< " La surface du triangle PQR vaut : " << surf(p,q,r) << std::endl;
- Je remplacerais "copyright pgl10" par "@author pgl10". Il n'y a pas de copyright sur un site de partage, je pense que tu voulais simplement mettre l'auteur.
J'ai émis beaucoup de remarques, mais ça ne remet pas en question la qualité du code présenté.