Discussion :

[killwin]

Bonjour, voilà c'est pour avoir un peu vos avis sur mon style de programmation, ce qu'il y a de bien ou de pas bien. La version 1.0 est terminée, mais je vais améliorer tout ça tout de même.

http://tuxworld.tuxfamily.org

[koala01]

Salut,

Bon, tu te douteras bien que je n'ai fait qu'effleurer une partie du code, mais:

En C++, il n'y a pas besoin de passer par le typedef struct, obligatoire en C, comme tu l'as fait pour toutes les structures que j'ai rencontrées.

La définition d'une structure sert en effet de déclaration pour le type en question, et tu peux donc parfaitement te contenter d'un simple

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
struct Color4f{
   float r, g, b, a;
} ;

au lieu d'un

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
typedef struct S_Color4f {
   float r, g, b, a;
} Color4f;

Je profites d'ailleurs de ce code pour te conseiller de respecter scrupuleusement la règle "une ligne, une variable".

Cela ne coute strictement rien d'écrire

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
struct Color4f{
   float r;
   float g;
   float b;
   float a;
} ;

mais, cela facilite grandement la relecture

Dans le même ordre d'idée, il faut savoir que les noms des variables n'influencent en rien le résultat final une fois le code compilé.

L'idéal est donc d'essayer de choisir en permanence des noms explicites, qui permettront de savoir à la simple lecture à quoi l'on a affaire...

Je sais bien que l'on parle de couleur "rgba", mais l'idéal aurait été d'utiliser les noms complets, à savoir

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
struct Color4f{
   float red;
   float green;
   float blue;
   float alpha;
} ;

Cela n'aurait pas couté grand chose, mais aurait facilité encore une fois la lecture

Ta classe CGraphic et toutes celles qui en dérivent devrait avoir un destruteur virtuel, car je présume que les différents objets sont gérés par allocation dynamique de la mémoire.

Si tu ne rend pas le destructeur virtuel,tu coures le risque d'une mauvaise destruction des objets de types dérivés, avec d'éventuelles fuites de mémoire très importantes

Je n'ai pas lu énormément de code à part celui des fichiers d'en-tête, mais j'ai remarqué certaines fonctions qui faisaient plus de 600 lignes, voire même près de 900 lignes pour l'une d'elles.

C'est douze à 20 fois trop!!!

l'idéal est de toujours essayer de limiter la taille des fonctions à environ 25 à 50 lignes grand maximum.

Bon, on peut ne pas chipoter pour une ou deux lignes de plus, mais là, nous sommes vraiment très loin de cette limite

Essayes de garder en permanence en tête le principe de la délégation des tâches : toute classe, toute fonction, ne devrait s'occuper que d'une chose, mais devrait le faire correctement

Il en va d'ailleurs de la lisibilité du code, car, lorsque tu dois parcourir 6 à 900 lignes de code, il devient très difficile de garder en tête la logique complete suivie par la fonction, en cas de débugage

De plus, une bonne délégation des responsabilités te permettra sans doute d'éviter de répéter 20 fois le même code, au besoin en passant certains paramètres en argument, et participera donc à la "simplification générale" de ce dernier .

Enfin, fais attention à la cohérence de tes règles de nommage : tu devrais éviter les règles qui font que tu en vienne a devoir utiliser des noms exclusivement en minuscules pour les fonction et à devoir jongler avec des majuscules et des minuscules pour les champs de certaines structures.

Idéalement les champs de structure, les variables membres et les fonctions membres devraient respecter les même conventions, au niveau de la casse à tout le moins

[PilloBuenaGente]

//Parenthèses

"Il me semble" qu'en passant par :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
typedef struct S_Color4f {
   float r, g, b, a;
} Color4f;

il est possible d’accéder à la structure en utilisant simplement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
    Color4f.r

au lieu de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
     struct Color4f.r

[koala01]

//Parenthèses

"Il me semble" qu'en passant par :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
typedef struct S_Color4f {
   float r, g, b, a;
} Color4f;

il est possible d’accéder à la structure en utilisant simplement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
    Color4f.r

au lieu de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
     struct Color4f.r

Ca, c'est vrai en C, mais, en C++ ca ne l'est plus:

Avec un code comme

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
struct Color4f{
   float r;
   float g;
   float b;
   float a;
} ;

tu peux parfaitement l'utiliser sous la forme de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
Color4f myColor;
myColor.r = /* une valeur */ ;

En C++, il n'y a que deux différences, assez maigres d'ailleurs, entre une structure et une classe:

1. la visibilité par défaut des membres d'une classe est private alors qu'elle est public pour les structures
2. La visibilité de l'héritage d'une classe est private alors qu'elle est public pour les structures

A ces deux détails près, l'utilisation de struct est strictement identique à celle de class, et tu pourrait d'ailleurs parfaitement envisager d'y rajouter un constructeur, voire des fonctions membres

A ce sujet, les types de données comme Color4f et autres ont, classiquement, sémantique de valeur!

Cela signifie qu'elles sont copiables, assignables, généralement comparables et... constante.

En effet, tu peux très bien avoir deux objets de type Color représentant des valeurs identique qui utilisent deux adresses mémoire différentes, par contre, si tu modifies une des valeurs dont elles sont composée, tu obtiens... une nouvelle couleur

[killwin]

Je ne savais pas pour struct, je vais changer ça.
Pour ma structure, elle n'a rien à faire ici, j'ai du l'écrire avant d'inclure OpenGL, car cette structure existe déjà sous OpenGL

Sinon pour le nomage des variables tu as raison à mon avis. J'avais un prof qui me disait de nommer explicitement les variables et les fonctions, et qu'un bon programme n'a pas besoin de commentaires, genre :

float Fonction_qui_multiplie_le_parametre_par_trois(float parametre);

Le débat est ouvert. Perso je pense qu'on se retrouve avec des lignes de code super longues mais explicites. :-)

Sinon tu as raison j'ai deux fichiers .cpp beaucoup trop longs. Je découpe ma grosse fonction principale en sous-fonctions inline qui ne sont appelées qu'une seule fois ?

Merci pour ton intervention et ta patience.

[koala01]

float Fonction_qui_multiplie_le_parametre_par_trois(float parametre);

Le débat est ouvert. Perso je pense qu'on se retrouve avec des lignes de code super longues mais explicites. :-)

Ben il est possible de trouver un "juste milieu"
multplyByThree, par exemple

Sinon tu as raison j'ai deux fichiers .cpp beaucoup trop longs. Je découpe ma grosse fonction principale en sous-fonctions inline qui ne sont appelées qu'une seule fois ?

Ce qui importe, surtout, c'est de respecter la règle de la responsabilité unique...

L'idée est que chaque fonction ne doit faire qu'une chose mais qu'elle doit la faire correctement

En respectant ce principe, il devient d'ailleurs beaucoup plus aisé de s'assurer que chaque fonction fait bien ce qu'elle est sensée faire en mettant des tests unitaires en place

Mais je ne crois pas que l'inlining systématique des fonctions n'apportera quoi que ce soit en terme de performances:

Déjà, l'appel de fonction ne demande que quelques instructions processeur, et s'effectue donc sur seulement quelques cycles d'horloges, ce qui est généralement peu par rapport au temps qu'il faut pour exécuter certaines boucles.

Ensuite, le fait de déclarer une fonction inline ne fait que demander au compilateur de remplacer l'appel de la fonction par le code correspondant, mais le compilateur reste en définitive seul juge pour décider de le faire ou non.

Il y a en effet de nombreux freins à sa capacité d'inlining, comme le fait qu'une fonction soit virtuelle ou qu'elle doive etre traduite en un nombre d'instructions processeurs supérieur à une certaine limite.

Enfin, l'inining peut etre considéré comme une optimisation prématurée, et devrait etre réservé, en gros, aux cas dans lesquels il est prouvé qu'il apporte réellement un gain significatif (mais cela signifie qu'il faut... trouver un moyen de comparer objectivement les performances avec et sans inlining )

[germinolegrand]

J'avais un prof qui me disait de nommer explicitement les variables et les fonctions, et qu'un bon programme n'a pas besoin de commentaires

Je ne peux que plussoyer ce prof.
Par contre l'exemple donné est totalement bidon car la logique même nous dit que cette fonction est inutile. Mais d'un autre côté, c'est la bonne nomenclature de la fonction qui nous signale que cette fonction est inutile, donc encore un bon point pour le nommage explicite

Pour ce qui est de l'inlining c'est pas compliqué : quand ta fonction contient simplement un return et qu'elle n'est pas virtuelle, alors elle devrait être inlinée afin d'éliminer une indirection inutile (sauf si cela devait créer des dépendances cycliques dans le .h auquel cas on s'abstiendra ).

Dans les autres cas, cela relève de l'optimisation.

[r0d]

Ce qui importe, surtout, c'est de respecter la règle de la responsabilité unique...

L'idée est que chaque fonction ne doit faire qu'une chose mais qu'elle doit la faire correctement

Cette règle n'a l'air de rien, mais c'est vraiment, AMHA, une 4 ou 5 règles les plus importantes en développement logiciel (pas qu'en c++ donc).

Je préfère l'appeler la "règle de l'opération unique et explicite": une fonction ne doit effectuer qu'une seule opération et son nom doit expliciter ce que fait cette fonction.

Donner une limite au nombre de ligne d'une fonction c'est bien pour donner un ordre d'idée aux grands débutants, mais cela ne ma parait pas très sérieux. On peut avoir par exemple une fonction qui effectue une requête SQL simple mais très longue (par exemple un select sur 200 champs).