projet terminé, avis

Version imprimable

24/12/2012, 08h48
killwin

projet terminé, avis

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
24/12/2012, 11h19
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:

1 2 3 struct Color4f{ float r, g, b, a; } ;
au lieu d'un
Code:

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:

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:

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 :D

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 ;)
24/12/2012, 16h20
PilloBuenaGente
//Parenthèses

"Il me semble" qu'en passant par :
Code:

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:

1 2 Color4f.r
au lieu de
Code:

1 2 struct Color4f.r
24/12/2012, 16h36
koala01
Citation:
Envoyé par PilloBuenaGente

//Parenthèses

"Il me semble" qu'en passant par :

Code:

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:

1 2 Color4f.r

au lieu de

Code:

1 2 struct Color4f.r
Ca, c'est vrai en C, mais, en C++ ca ne l'est plus:

Avec un code comme
Code:

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:

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 ;)
25/12/2012, 22h49
killwin

merci

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. :mouarf:
26/12/2012, 07h35
koala01

Citation:

Envoyé par killwin

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 :D

Citation:

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 ;) )
26/12/2012, 12h20
germinolegrand

Citation:

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 :D).

Dans les autres cas, cela relève de l'optimisation.
26/12/2012, 16h23
r0d

Citation:

Envoyé par koala01

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).
26/12/2012, 16h26
germinolegrand

SQL est si pourrie qu'il faille écrire autant de lignes que de requêtes ???
Si c'est vraiment le cas, il faut exploiter les macros ^^ écrire 200 lignes de copier collé ou presque me révulserait au plus haut point...

edit : ah et 200 lignes c'est pas ce que j'appellerais une fonction trop longue, tout dépend de ce qu'elle fait.
26/12/2012, 16h33
r0d
De même que la règle: une ligne = une opération (qui n'est qu'une spécialisation de la règle précédente) est une bonne règle.

Par exemple:
Code:

1 2 3 4 inline void CAnimation::pass(const int& loop_type, const int& step) { // we go to the next image up_down ? cursor -= step : cursor += step; // argh
Je déteste cette notation. C'est certes compact, mais ça demande un inutile effort de compréhension pour l'analyse.
Code:

1 2 #ifndef ANIMATION #define ANIMATION
Conseil: utilise des guards un peu plus complexes, ça peu éviter de mauvaises surprises. Par exemple:
Code:

1 2 #ifndef ANIMATION_H__ #define ANIMATION_H__
Code:

1 2 3 4 5 enum { NO_LOOP, LOOP, UP_DOWN_LOOP };
les capitales sont généralement réservées pour les #define (et donc les macros également).

Sinon, le code est propre et bien documenté, ça fait plaisir :ccool:
Pas grand chose à dire donc (je ne suis pas plongé en détail non plus). Juste peut-être quelque chose d'étrange (mais il y a peut-être une raison): il n'y a aucune fonction membre constante.
26/12/2012, 16h36
r0d

Citation:

Envoyé par germinolegrand

SQL est si pourrie qu'il faille écrire autant de lignes que de requêtes ???

Ben, SQL est comme tout langage: il est ce qu'on en fait.
Lorsqu'une base de donnée est mal conçue, on ne pourra faire que des trucs pourris pour l'attaquer, surtout lorsqu'on doit passer par un wrapper.
26/12/2012, 17h16
koala01
Citation:

Envoyé par r0d

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 le nom de cette fonction 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).

De toutes manières, personne n'est vraiment d'accord sur le nombre de ligne maximal que l'on peut admettre ;)

Mais une réminiscence de l'époque où l'affichage se faisait sur 25 lignes de 80 colonnes mène régulièrement à considérer que 50 lignes (l'équivalent de deux écrans d'affichages) représentent une limite sensée ;)

Je n'engueulerai jamais quelqu'un qui fournit une fonction qui ne fait effectivement qu'une seule chose mais qui la fait en 60 lignes, mais j'engueulerai surement quelqu'un qui fournit une fonction de 40 lignes et qui prend trois responsabilités :D

Citation:

Envoyé par germinolegrand

SQL est si pourrie qu'il faille écrire autant de lignes que de requêtes ???
Si c'est vraiment le cas, il faut exploiter les macros ^^ écrire 200 lignes de copier collé ou presque me révulserait au plus haut point...

Tout dépend du select que tu fais, mais si tu dois t'amuser avec des jonctions, des requetes qui te permettront de sélectionner une série d'identifiants, ou d'autre trucs du genre, on en arrive assez facilement à des requete imbitables :D

Ceci dit, l'idéal reste quand meme de "factoriser" le code de cette requete, en ayant, pourquoi pas, plusieurs fonctions qui créent des parties spécifiques

Citation:

edit : ah et 200 lignes c'est pas ce que j'appellerais une fonction trop longue, tout dépend de ce qu'elle fait.

S'il y a une chose à savoir en informatique, c'est que de nombreuses règles doivent etre comprises suffisamment bien pour savoir quand il est bon d'y déroger ;)

Mais il n'empêche que j'aurai quand meme toujours tendance à considérer a priori une fonction de 200 lignes avec énormément de suspicion, du moins, jusqu'à ce que j'aie la preuve formelle qu'il n'était effectivement pas possible de faire autrement ;)

Même avec une incrémentation correcte, je trouve que la complexité de compréhension d'une fonction s'élève de manière quasi exponentielle par rapport à son nombre de lignes, et je serais vraiment très étonné qu'il n'y ait pas de possibilités élégantes de factoriser quelque chose qui demande 200 lignes de code ;)
[quote:r0d]
De même que la règle: une ligne = une opération (qui n'est qu'une spécialisation de la règle précédente) est une bonne règle.

Par exemple:
Code:

1 2 3 4 inline void CAnimation::pass(const int& loop_type, const int& step) { // we go to the next image up_down ? cursor -= step : cursor += step; // argh
[/quote]Tout à fait : ce n'est qu'un sucre syntaxique qui n'apporte rien, en terme de performances à la version "if ... else" classique qui serait (pour reprendre l'exemple) proche de
Code:

1 2 3 4 if(up_down) cursor-=step; else cursor+= step;
dont la compréhensibilité est malgré tout largement meilleure.

Cependant, toute regle ne vaut que par les exceptions qu'on admet à son sujet, et il existe en effet quelques cas, très rares au demeurant, dans lesquels l'utilisation d'un if... else demanderait une gymnastique que l'opérateur ternaire ne demanderait pas, et dans lesquels l'opérateur ternaire se justifie donc pleinement ;)

En outre, l'opérateur ternaire peut être évalué à la compilation, alors que le if... else classique ne peut pas l'être.

Lorsqu'on programme en suivant le paradigme generique, c'est parfois la seule solution dont on dispose pour pouvoir "activer" ou non des capacités contradictoires ;)

Mais bon, ca, c'est des cas réellement particuliers ;)
26/12/2012, 17h42
germinolegrand

Pour le ternaire, j'aurais tendance à écrire celui-ci de cette façon :

Code:

cursor += (up_down ? -1 : +1)*step;

Ce qui m'évite toute redondance d'identificateur => refactorisation future optimisée, lecture du code optimisée.

Le problème soulevé par le SRP (Single Responsability Principe), c'est la délimitation de la responsabilité, qui est abstraite, par conséquent floue.

A partir de quand considère-t-on qu'une fonction/classe prend 2 responsabilités ? Je pense qu'un tel débat peut avoir un intérêt énorme, à mes yeux en tout cas.
27/12/2012, 08h25
killwin

ternaire

Je commence déjà à réécrire mon code. :-) en prenant en compte vos remarques. Vous me donnez du boulot ;)

Citation:

cursor += (up_down ? -1 : +1)*step;

La lecture est meilleure, mais ça rajoute tout de même une super multiplication.

J'ai compris un truc en informatique,
factoriser = calcul plus long, mais souvent meilleure lisibilité. (parfois c'est l'effet inverse)

Je sais que les machines sont puissantes de nos jours, mais personnellement je me pose la question ? J'écris pour l'homme ou pour la machine ?

Par exemple quand je divise x par 8 et que x est un entier, hop, décalage de bits, et si le gars qui passe derrière moi ne comprends pas, je me dis qu'il n'avait qu'à savoir lire. :P

Mais j'ai aussi moi même du mal à me relire parfois

!(y - 1) ? (x ? y << 1: y >> 2) : x << 1
27/12/2012, 09h11
kinaesthesia

Citation:

Envoyé par killwin

Je sais que les machines sont puissantes de nos jours, mais personnellement je me pose la question ? J'écris pour l'homme ou pour la machine ?

Par exemple quand je divise x par 8 et que x est un entier, hop, décalage de bits, et si le gars qui passe derrière moi ne comprends pas, je me dis qu'il n'avait qu'à savoir lire. :P

Le compilateur peut très bien optimiser ce que tu ne pensais pas pouvoir être optimiser, bref.

Le problème pour moi est que ton "8" est un magic number. Tu devrais lui donner un vrai nom explicite car pour la machine un nom de variable ou une variable globale avec un nom correct est pareil.
27/12/2012, 09h22
r0d

En effet, il faut bien comprendre que les bons compilateurs aujourd'hui optimisent beaucoup de choses. Le décalage de bit c'est la base, mais ça peut aller très loin.

Aujourd'hui, l'optimisation ça ne consiste plus du tout à essayer de gagner 2 cycles sur une opération (ça en général le compilateur le fait mieux que nous), mais utiliser les conteneurs adéquats, éviter les copies inutiles, chercher des heuristiques malines, utiliser au mieux les ressources à disposition (multithreading, grid, etc.), ce genre de choses.
27/12/2012, 09h28
koala01
Citation:

Envoyé par germinolegrand

A partir de quand considère-t-on qu'une fonction/classe prend 2 responsabilités ? Je pense qu'un tel débat peut avoir un intérêt énorme, à mes yeux en tout cas.

Je dirais, de manière générale, dés qu'il te faut plus d'un verbe pour expliquer la logique de ta fonction:
- Si ta fonction commence par trier les données avant de sélectionner celles qui correspondent à un critère donné,
- si elle doit charger des données avant de les afficher,
- si elle doit calculer quelque chose avant d'utiliser le résultat,
- si elle doit demander quelque chose avant de réagir en fonction de l'introduction de l'utilisateur,
c'est chaque foisque ta fonction a d'office au minimum deux responsabilités, et donc une de trop ;)

Lorsque j'essaye d'expliquer comment essayer de déterminer ce qu'il faut mettre en place lors d'une conception, je dis généralement de partir du principe qu'il faut exprimer clairement ce dont on a besoin et partir du principe que les noms représentent des types qu'il faudra implémenter et que les verbes représentent des comportements (fonctions libres ou fonctions membre).

Cette manière de travailler est, certes, perfectible et sans doute un peu à l'emporte pièce, mais c'est, en tous cas, une base saine pour envisager les choses ;)

Citation:

Envoyé par killwin

La lecture est meilleure, mais ça rajoute tout de même une super multiplication.

En nombre de lignes, cela reviendra quasiment au même ;)

Citation:

J'ai compris un truc en informatique,
factoriser = calcul plus long, mais souvent meilleure lisibilité. (parfois c'est l'effet inverse)

Pourquoi un calcul plus long :question:

Au contraire, le fait de pouvoir séparer les différentes étapes importantes te permettent de ne t'inquiéter que d'un problème à la fois, et donc d'avoir des problèmes plus simples à résoudre, impliquant des solutions plus simples elles aussi ;)

Citation:

Je sais que les machines sont puissantes de nos jours, mais personnellement je me pose la question ? J'écris pour l'homme ou pour la machine ?

Tu écris d'abord pour l'homme, puis pour la machine.

Il faut te dire que ton code sera beaucoup plus souvent lu / modifié qu'il ne sera compilé et que, s'il est trop complexe, tu ne sera pas en mesure de corriger facilement les éventuelles erreurs.

De plus, les jeux d'instructions au niveaux des processeurs sont tels que deux codes ayant un comportement identique dont l'un utilise des sucres syntaxiques seront traduits exactement de la même façon dans l'exécutable final.

Simplement, l'un sera plus "reader friendly" que l'autre ;)

Citation:

Par exemple quand je divise x par 8 et que x est un entier, hop, décalage de bits, et si le gars qui passe derrière moi ne comprends pas, je me dis qu'il n'avait qu'à savoir lire. :P

Il ne s'agit pas de "savoir lire" ou non, il s'agit de l'effort mental nécessaire à la compréhension d'un ensemble d'instruction.

Tu auras beaucoup plus facile à comprendre i *= 8 que i <<3, parce que, dans le deuxième cas, tu devra avoir une "gymnastique mentale" plus importante (en devant déjà commencer par calculer la valeur de 2^nombre de bits de décalage)

Citation:

Mais j'ai aussi moi même du mal à me relire parfois

!(y - 1) ? (x ? y << 1: y >> 2) : x << 1

C'est sans doute la meilleure preuve que ton écriture n'est pas bonne ;)

Si tu as déjà du mal à te relire toi-même alors que tu es sans doute la personne la plus à mène de savoir ce que tu voulais faire, dis toi que quelqu'un qui découvre ton code sans savoir ce que tu pouvais avoir en tête au moment où tu l'as écrit a de grandes chances d'avoir énormément de mal à le comprendre ;)

Ce qu'il faut savoir au niveau des sucres syntaxiques, c'est qu'ils n'ont été mis au point que parce que, à une époque, il y avait de très fortes contraintes techniques en terme d'affichage et de visibilité du code (les écrans, par exemples, n'étaient capables que d'afficher 25 lignes de 80 colonnes).

Il fallait donc, effectivement, trouver le moyen de faire tenir "un maximum d'information" sur "un minimum d'espace".

Ces contraintes techniques ont été levées, en gros, depuis l'époque de windows 3 / 3.1 et l'arrivée d'éditeur de texte pouvant sans problème afficher, sur un écran, largement plus de 25 lignes et 80 colonnes.

Le besoin de sucre syntaxique a donc simplement diminué en même temps que les contraintes techniques, et, comme il n'y a pas de différence au final en terme de code binaire généré, il est largement préférable de privilégier la relecture aisée du code ;)

Personnellement, je rève du jour où C et C++ se décideront à supprimer l'opérateur , pour la déclaration de variables, par exemple ;)
27/12/2012, 09h43
r0d
Citation:

Envoyé par koala01

Je dirais, de manière générale, dés qu'il te faut plus d'un verbe pour expliquer la logique de ta fonction:

Pas mal comme définition :ccool:

Cependant, j'ai peur que ce ne soit pas aussi simple :aie:
Prenons un exemple concret: on a un gros objet plein de variables et on veut le sérialiser. Le sauvegarder dans un fichier xml disons. On veut écrire une fonction exportToXmlFile( ofstream & file ), par exemple (il y a mille façons de serialiser un objet). Et bien cette fonction peut faire 500 lignes juste en faisant des
Code:

1 2 3 4 file << "<root>" << endl; file << "<variables>" << endl; file << "<variable>" << ma_variable << "</variable>" << endl; // etc.
Pourtant la fonction ne fera qu'une seule chose: sauvegarder notre objet au format xml.

Mai peut-être devrions-nous ouvrir un autre fil?
27/12/2012, 10h00
koala01
Citation:
Envoyé par r0d

Pas mal comme définition :ccool:

Cependant, j'ai peur que ce ne soit pas aussi simple :aie:
Prenons un exemple concret: on a un gros objet plein de variables et on veut le sérialiser. Le sauvegarder dans un fichier xml disons. On veut écrire une fonction exportToXmlFile( ofstream & file ), par exemple (il y a mille façons de serialiser un objet). Et bien cette fonction peut faire 500 lignes juste en faisant des

Code:

1 2 3 4 file << "<root>" << endl; file << "<variables>" << endl; file << "<variable>" << ma_variable << "</variable>" << endl; // etc.

Pourtant la fonction ne fera qu'une seule chose: sauvegarder notre objet au format xml.

Mai peut-être devrions-nous ouvrir un autre fil?
J'ai bien dit que la méthode est perfectible, et qu'elle se contente de donner une base de départ pour la réflexion.

Cependant, tu remarquera que j'ai parlé de décrire la logique qui est implémentée dans la fonction ;)

Et donc, si l'on reprend ton exemple, nous pourrions déjà décrire la logique sous la forme de
1. écrire l'en-tete du fichier
2. "ouvrir" la racine
3. écrire les données
4. "fermer la racine"
Mon dieu! cela fait quatre verbes... donc trois de trop ;)

Cela nécessite donc trois fonctions
createXmlHeader : qui écrit les fameuses lignes qui indiquent l'encodage
openRootNode qui contient
Code:

1 2 3 file << "<root>" << endl; file << "<variables>" << endl;
writeData, qui mérite peut etre d'être également factorisée ;)
closeRootNode qui contient
Code:

1 2 3 file << "</variables>" << endl file << "</root>" << endl;;
et donc, une fonction exportToXmlFile( ofstream & file) qui prend la forme de
Code:

1 2 3 4 5 6 7 exportToXmlFile( ofstream & file) { createXmlHeader(file); openRootNode(file); writeData(file); closeRootNode(file); }
mon conseil est donc parfaitement respecté et respectable :d
27/12/2012, 16h45
germinolegrand

Définition intéressante, mais qui amène quand même un problème :D Si on la suit jusqu'au bout, on ne fera qu'écrire des fonctions contenant des appels à des fonctions... On finit par perdre un temps énorme en sous-découpage.
J'ai l'impression que c'est un peu pousser l'idée à l’extrémisme...

A mon avis il faut trouver un juste milieu, et pour moi tant qu'on n'écris un code qu'une seule fois, qu'il fasse 4 trucs différents je m'en fous, tant que le nommage de la fonction est explicite ça regarde pas l'utilisateur.
Dès qu'on doit réutiliser un bout du code, hop on l'encapsule.
27/12/2012, 19h10
backlash

Citation:

et pour moi tant qu'on n'écris un code qu'une seule fois, qu'il fasse 4 trucs différents je m'en fous

Donc si N (choisir N très grand) lignes de code ne sont utilisées qu'une fois, on ne découpe pas.

Ainsi le jour où tu t'aperçois (si tu t'en aperçois) qu'il y a un bug dans ces N lignes, tu maudiras le jour où tu t'es fixé cette règle (si on peut appeler ça une règle) à cause du temps que tu auras perdu en débuggage.

Pire encore comment mettras tu en place les test unitaires de ces N lignes sachant que tu dois couvrir la totalité du code avec ces tests. Cela deviendra vite très compliqués à identifier les bon tests pour les différentes responsabilités qui auront été attribué à ces N lignes.

Dès lors il apparait évident que tes tests auront du mal à couvrir entièrement ton code aux multiples responsabilités, par conséquent ton intégration continu en devient moins stable (pour ne pas dire instable).

Ce qui aura pour simple effet de mener le projet à sa perte.
27/12/2012, 21h24
koala01
Citation:

Envoyé par germinolegrand

Définition intéressante, mais qui amène quand même un problème :D Si on la suit jusqu'au bout, on ne fera qu'écrire des fonctions contenant des appels à des fonctions... On finit par perdre un temps énorme en sous-découpage.

Je ne vois absolument pas où est le problème d'avoir une fonction qui va en appeler trois autre qui auront les responsabilité d'initialisation, d'exécution et de finalisation et d'avoir, dans ces fonctions importante, encore autant d'appels à des fonctions qui prennent en charge une partie bien spécifique.

Au contraire, cela te permet de ne t'attaquer qu'à un problème à chaque fois sans être distrait par l'arbre qui cache la forêt ;)

Citation:

A mon avis il faut trouver un juste milieu, et pour moi tant qu'on n'écris un code qu'une seule fois, qu'il fasse 4 trucs différents je m'en fous, tant que le nommage de la fonction est explicite ça regarde pas l'utilisateur.
Dès qu'on doit réutiliser un bout du code, hop on l'encapsule.

Je vois trois raisons de ne pas le faire de la sorte:

La première, c'est que, avec l'augmentation de ta base de code, il deviendra rapidement très difficile de repérer les endroits où tu duplique du code et de l'encapsuler a posteriori .

La deuxième, c'est que, comme il s'agit de donner des noms explicites à tes fonctions, si elles font plusieurs chose, soit tu va te retrouver avec des noms à rallonges (du genre sortItemsThenSelectSomething), soit tu va "mentir" à l'utilisateur sur ce que la fonction fait réellement (par exemple, en cachant à l'utilisateur que l'appel d'une fonction va changer la structure interne de ton objet en triant les objets qu'il contient)

La troisième est que tu pourras beaucoup plus facilement déclarer une fonction constante si tu es sur qu'elle ne fait effectivement qu'une seule chose, alors qu'il sera difficile de déclarer ta fonction sortItemsThenSelectSomething (juste pour avoir un exemple ;)) comme telle.

Au final, tu facilite le travail de l'utilisateur des différentes fonctions (c'est à dire souvent toi meme ;)) en partant du principe que tu ne doit pas utiliser plus d'un verbe pour pouvoir expliquer la logique qui sous tend à une fonction, en lui (t' ) évitant la mauvaise surprise d'une erreur du style " l'appel à xxx casse la const correctness" ou d'un pointeur /d'une référence rendu(e) caduque par une fonction qui n'indiquait pas clairement qu'elle allait l'invalider ;)

Je peux t'assurer que je n'ai jamais eu de problème avec du code dans lequel le principe est appliqué de manière systématique, alors que, chaque fois qu'un problème est apparu, c'est, justement, parce qu'une fonction se retrouvait en définitive à en faire plus que ce qu'elle ne voulait le laisser croire.

Quand tu "débarques" sur un projet qui est déjà composé de plusieurs milliers de fichiers (ce qui te donne une idée du nombre de lignes déjà écrites et de fonctions que cela peut représenter :D ), il t'est totalement impossible d'envisager de passer ton temps à parcourir chaque ligne de code pour t'assurer que chaque fonction fait ce qu'elle prétend, ni plus ni moins.

Tu vas donc partir du principe que chaque fonction ne fait... que ce qu'elle indique de par son nom ;)

Lorsque tu apportes une évolution au projet, tu vas continuer à te baser sur le fait que la fonction ne fait ni plus ni moins que ce qu'elle prétend.

Si la fonction que tu envisages d'utiliser pour l'usage que tu en as en fait plus que ce qu'elle ne prétend, c'est tout le chateau de cartes qui s'effondre car tu seras rapidement confronté au fait qu'un des "effets secondaires" de ta fonction n'est pas souhaitable ni souhaité pour ton usage particulier.

Tu te retrouves donc souvent à devoir refactoriser une fonction, non pas pour être en mesure de n'appeler un comportement qui ne correspond qu'à une partie spécifique dont tu as besoin, mais au contraire pour pouvoir récupérer ... quasiment tout ce qu'elle fait, à l'exception d'une petite partie qui ne te convient pas.

Et toute refactorisation demande beaucoup plus de temps à la mise en oeuvre que ce que cela ne t'aurait demandé pour correctement séparer les responsabilités au moment où tu as écrit ton code ;)

Imaginons que tu aies une fonction sous la forme de
Code:

1 2 3 4 5 6 7 8 void foo() { stuffOne(); stuffTwo(); stuffThree(); stuffFour(); stuffFive(); }
Et que tu te dises, à lire le nom (qui serait explicite sur ce qui est fait, contrairement à l'exemple :D) que cette fonction est "justement celle qui t'intéresse".

Tu vas faire confiance au nom de la fonction et utiliser foo dans ton propre code, en partant du principe qu'elle fait exactement ce qu'il faut.

Le problème, c'est que, dans le cas présent, stuffOne et stuffTwo sont deux étapes qui ne t'intéressent absolument pas.

Tu obtiens donc un résultat qui ne correspond absolument pas à celui auquel tu t'attendais en appelant foo :aie:

Ce que tu vas sans doute faire, c'est entrer dans le code de foo afin de déterminer pourquoi tu n'obtiens pas le résultat attendu.

Si le code est déjà factorisé, il te devient facile de remplacer l'appel à foo par les appels successifs à stuffThree, stuffFour et stuffFive (vu que j'ai dit que les deux premières étapes ne t'intéressent pas), et tu vas arriver au résultat voulu en moins de deux minutes, sans avoir eu à te pencher sur un nombre important de lignes de codes.

Par contre, si le code des cinq comportements représentés par stuffOne, stuffTwo, stuffThree, stuffFour et stuffFive se retrouve dans ta fonction foo, ca va être beaucoup plus difficile :aie:

En effet, tu vas déjà devoir passer un temps impressionnant à étudier chaque ligne de code pour avoir une vue d'ensemble de ce que fait la fonction.

Mais, en plus, une fois que tu auras déterminé que les lignes 1 à 30 ne t'intéressent pas car ce sont celles qui correspondent à stuffOne et stuffTwo, tu vas sans doute "simplement" factoriser dans une fonction séparée les lignes 31 à la fin, car ce sont celles qui t'intéressent.

Le problème, c'est que tu te retrouves néanmoins encore avec des fonctions qui ont plus d'une responsabilité, et que tu devras peut etre (sans doute) refaire exactement la même chose dans trois mois pour récupérer le seul comportement correspondant à stuffOne (ou à n'importe quel autre) et ainsi de suite.

Au final, cela va te prendre peut etre 5 ou 7 fois une demi heure à une heure de travail pour extraire chaque fois les comportements différents, avec tous les problèmes que peut impliquer une refactorisation du code, alors que cela ne t'aurait sans doute pas demandé cinq minutes supplémentaires de créer directement les cinq fonctions ayant chacune une responsabilité unique et clairement définie.

Si tu fais le compte du temps nécessaire à un découpage correct, le fait de s'y atteler directement va nous couter très certainement dix fois moins de temps que le fait de se dire "bah, tant pis si ma fonction fait en réalité trois choses différentes, on verra plus tard" ;)

Je n'ai pas honte de le dire, je suis de nature paresseuse : j'ai horreur de devoir refaire quelque chose qui aurait du être fait correctement dés le départ... Et toi, aimes tu tant que cela repasser sur les crasses que d'autres t'ont laissées :question:
27/12/2012, 23h26
germinolegrand

Le comportement interne de ma classe ne regarde certainement pas l'utilisateur... S'il est nécessaire de trier pour sélectionner, c'est le créateur que ça regarde, certainement pas l'utilisateur, et dans ce cas ma fonction s'appelle select() ^^

Si le créateur souhaite ajouter stuffOne() à l'interface publique de sa classe, c'est qu'il en a plusieurs fois l'utilité, et qu'il doit par conséquent bien factoriser son contenu en créant stuffOne().

Tant que l'état de l'objet est cohérent avec l'utilisation qu'en a eu l'utilisateur, alors la classe a parfaitement rempli son rôle, qu'importe la tête du code.

Deuxièmement, j'ai l'impression à vous entendre que lire et comprendre du code comportant plus que des appels à des fonctions est chose impossible... pourquoi ? Vous codez si mal que ça que c'en est illisible ??? J'en doute. Un code bien écrit se lit comme du français... ou plutôt de l'anglais :mouarf: .

Troisièmement, un couper-collé ça vous parait long comme temps de refactorisation ???
Ce n'est pas parce que ma fonction n'est pas découpée en sous-fonctions qu'elle doit être codée salement. On déclare ses objets dans l'ordre et on exploite le RAII.

Citation:

Ainsi le jour où tu t'aperçois (si tu t'en aperçois) qu'il y a un bug dans ces N lignes, tu maudiras le jour où tu t'es fixé cette règle (si on peut appeler ça une règle) à cause du temps que tu auras perdu en débuggage.

Pourquoi ? Si on arrive à écrire 2000 lignes d'affilée sans faire de répétition (ce dont je doute très fortement), je n'ai qu'à poser des breakpoints le long de ces lignes de code de la même façon que je les aurais disposées à l'entrée de sous-fonctions, puisque de toute façon qu'on fasse des sous-fonctions ou non, le code est linéaire et les sous-fonctions ne seraient appelées qu'une seule fois.

Citation:

Personnellement, je rève du jour où C et C++ se décideront à supprimer l'opérateur , pour la déclaration de variables, par exemple

Ce jour là je pleurerai certainement. Devoir répéter un type autant de fois qu'on a de variables de ce type et qui se rapportent à un même problème est juste... révulsant.
En effet, en déclarant des variables de la sorte, on indique ainsi clairement notre volonté d'avoir le même type pour un ensemble de variable, ici r,g,b,a si je me souvient bien. Comme d'habitude, on est gagnant si l'on souhaite faire du changement de type, il n'y a qu'un seul mot à changer (cf tous les avantages de la méta-programmation, vais pas faire un laïus dessus :P).

Ne me faites pas dire ce que je n'ai pas dis, il ne faut pas à tout prix réunir les variables d'un même type sur la même ligne, loin s'en faut, ceci s'applique seulement aux variables qui doivent toujours sémantiquement avoir le même type quel qu'il soit.
27/12/2012, 23h50
koala01

Citation:

Envoyé par germinolegrand

Le comportement interne de ma classe ne regarde certainement pas l'utilisateur... S'il est nécessaire de trier pour sélectionner, c'est le créateur que ça regarde, certainement pas l'utilisateur, et dans ce cas ma fonction s'appelle select() ^^

Tout dépend de la manière dont l'utilisateur a utilisé la classe, et surtout, comment il a pu utiliser les objets que la classe expose.

Et ca regarde surtout l'utilisateur de la classe si la fonction en vient à invalider certains de ces objets (ou plutot certains pointeurs / références sur ces dernier)

Citation:

Si le créateur souhaite ajouter stuffOne() à l'interface publique de sa classe, c'est qu'il en a plusieurs fois l'utilité, et qu'il doit par conséquent bien factoriser son contenu en créant stuffOne().

Mais qu'est ce qui l'empêche de factoriser directement ce comportement :question:

Au contraire, il a tout à gagner à le faire dans le sens où c'est comme avec le sel en cuisine : il est toujours plus facile de rajouter quelque chose que de le retirer ;)

Citation:

Tant que l'état de l'objet est cohérent avec l'utilisation qu'en a eu l'utilisateur, alors la classe a parfaitement rempli son rôle, qu'importe la tête du code.

Cela se discute très fort.

Bien sur, l'état de l'objet se doit de rester cohérent, mais l'idée est aussi de permettre une évolution vers une utilisation qui n'avait pas forcément été prévue au départ.

Et s'il faut en arriver passer des heures à refactoriser quelque chose, c'est du temps perdu bêtement ;)

Citation:

Deuxièmement, j'ai l'impression à vous entendre que lire et comprendre du code comportant plus que des appels à des fonctions est chose impossible... pourquoi ? Vous codez si mal que ça que c'en est illisible ??? J'en doute. Un code bien écrit se lit comme du français... ou plutôt de l'anglais :mouarf: .

Selon moi, la difficulté de "comprendre" le code d'une fonction évolue de manière exponentielle par rapport au nombre d'instructions qu'elle contient.

En gardant (au besoin un grand nombre) des petites fonctions simple, on s'évite bien des soucis ;)

Citation:

Troisièmement, un couper-collé ça vous parait long comme temps de refactorisation ???

Quand tu te souviens encore du code parce que tu viens de l'écrire, cela peut effectivement se résumer à cela..

Mais, quand c'est un code que tu as eu le temps d'oublier, parce que tu t'es occupé d'autre chose, ou pire, quand c'est un code que tu n'as pas écrit toi-même, ce n'est pas aussi évident que cela.

Citation:

Ce n'est pas parce que ma fonction n'est pas découpée en sous-fonctions qu'elle doit être codée salement. On déclare ses objets dans l'ordre et on exploite le RAII.

... Et l'on passe un temps bête à repérer les différentes étapes qui auraient pu être factorisées directement, ou à se demander ce qu'il faudra transmettre en argument pour refactoriser la fonction

Citation:

Pourquoi ? Si on arrive à écrire 2000 lignes d'affilée sans faire de répétition (ce dont je doute très fortement), je n'ai qu'à poser des breakpoints le long de ces lignes de code de la même façon que je les aurais disposées à l'entrée de sous-fonctions, puisque de toute façon qu'on fasse des sous-fonctions ou non, le code est linéaire et les sous-fonctions ne seraient appelées qu'une seule fois.

Si ce n'est, encore une fois, que, tout comme il est plus facile de lire du texte s'il est correctement divisé en paragraphe, il sera beaucoup plus facile de lire le code s'il est correctement divisé en fonction simples et concises ;)

Citation:

Ce jour là je pleurerai certainement. Devoir répéter un type autant de fois qu'on a de variables de ce type et qui se rapportent à un même problème est juste... révulsant.

Mais cela t'assure au moins de n'être pas dans une position où tu te demandes d'où vient une variable parce qu'elle est noyée entre une multitude d'autres ;)

Citation:

En effet, en déclarant des variables de la sorte, on indique ainsi clairement notre volonté d'avoir le même type pour un ensemble de variable, ici r,g,b,a si je me souvient bien. Comme d'habitude, on est gagnant si l'on souhaite faire du changement de type, il n'y a qu'un seul mot à changer (cf tous les avantages de la méta-programmation, vais pas faire un laïus dessus :P).

Un bien faible avantage en comparaison des risques encourus ;)
28/12/2012, 10h14
killwin

private, public

Citation:

Le comportement interne de ma classe ne regarde certainement pas l'utilisateur... S'il est nécessaire de trier pour sélectionner, c'est le créateur que ça regarde, certainement pas l'utilisateur, et dans ce cas ma fonction s'appelle select() ^^

D'où les private, public, protected. Mais en language Python ce qui est incroyable c'est que par phylosophie tout est 'public' puisqu'ils partent du principe qu'on est pas des enfants et que tout le monde est responsable dans un projet.
28/12/2012, 13h52
Klaim

Citation:

Mais en language Python ce qui est incroyable c'est que par phylosophie tout est 'public' puisqu'ils partent du principe qu'on est pas des enfants et que tout le monde est responsable dans un projet.

Ce n'est pas tout a fait vrai, tout est publique mais tu peux cacher des fonctions.