Classe simple pour produire un hash sha-1 d'un buffer quelconque
xbabilone
Messages postés
47
Date d'inscription
vendredi 18 février 2005
Statut
Membre
Dernière intervention
7 janvier 2018
-
2 janv. 2015 à 21:43
cptpingu Messages postés 3837 Date d'inscription dimanche 12 décembre 2004 Statut Modérateur Dernière intervention 28 mars 2023 - 16 janv. 2015 à 10:27
cptpingu Messages postés 3837 Date d'inscription dimanche 12 décembre 2004 Statut Modérateur Dernière intervention 28 mars 2023 - 16 janv. 2015 à 10:27
Cette discussion concerne un article du site. Pour la consulter dans son contexte d'origine, cliquez sur le lien ci-dessous.
https://codes-sources.commentcamarche.net/source/100864-classe-simple-pour-produire-un-hash-sha-1-d-un-buffer-quelconque
https://codes-sources.commentcamarche.net/source/100864-classe-simple-pour-produire-un-hash-sha-1-d-un-buffer-quelconque
16 janv. 2015 à 10:27
À voir ce que donnerait le design à ce moment là. Je pense néanmoins, qu'une fonction supplémentaire serait plus adaptée qu'une classe, vu que les méthodes actuelles n'utilisant pas d'attributs de classe, rajouter une nouvelle méthode ne changerait pas fondamentalement le design (ou très peu, 90% de ton code ne serait pas dans ta classe, mais en fonctions "anonymes").
C'est beaucoup plus clair. Cette explication devrait être dans ta description, avec lien vers la doc :)
14 janv. 2015 à 23:33
Tout d'abord, je me suis permis de remettre à zéro la conversation. La précédente n'étant plus nécessaire.
Au niveau du code, il y a beaucoup à dire. Je vais découper ma critique en plusieurs domaines.
== Au niveau du design de code ==
- Tu as crée une classe, mais tu ne te sers absolument pas des propriétés d'une classe !
Il serait donc mieux de créer une fonction définit dans le header. Toutes les méthodes privées de ta classe seraient alors des fonctions uniquement présent dans le fichier de code.
Tu peux créer un namespace "Hash" autour de ta fonction. On ne crée de méthode que si celle-ci doit accéder à un attribut de la classe. Sinon, on en fait une fonction privée dans un namespace anonyme, au sein du fichier de code.
- Vu que ton code dépend fortement de la bonne taille des entiers, utilise des entiers à taille fixe, comme uint32_t, int32_t, int64_t et uint64_t qui sont inclus dans <cstdint>. Il est généralement conseillé de les utiliser en lieu et place de int, long, etc...
- Il faut mieux découper ton code. Toujours découper des "idées" en plus petites "idées". Plutôt qu'une grosse fonction géante, il est préférable d'avoir plein de petites fonctions. Une fonction ne devrait pas excéder 50 lignes (la plupart du temps).
- Il faut vraiment mieux nommer tes variables ! La majorité ne sont que des lettres dénuées de sens: (a, b, c, d, e, f c'est vraiment horrible). N'oublie pas que quelqu'un qui te relit n'est pas forcément dans ta tête et ne comprendra pas clairement si tes noms variables sont trop cryptiques. (Ca vaut ausi pour toi si tu te relis dans 6 mois :p).
- En C++, une bonne API ne prend pas du char*, mais du std::string (surtout que le char* rentre tout seul dans le std::string). Ta fonction principale hash() devrait donc prendre un std::string en entrée.
- Ne met pas ton code dans le header. Le header ne sert qu'à déclarer du code, mais pas à en implémenter. Il faut toujours un fichier de code avec un fichier de header. De plus, ça te permet aussi de recompler plus vite, vu que les .cc déjà compilés qui ne sont pas modifiés ne sont pas recompilé à nouveau, au contraire des .hh (sur de gros projet, ça fait la différence).
== Au niveau technique ==
- C'est un grand classique, qu'on s'échine a répéter inlassablement: Evite les using namespace ! Voir: http://0217021.free.fr/portfolio/axel.berardino/articles/bon-usage-using-namespace
- Il est préférable d'inclure <cmath>, <cstring>, <cstdlib>.
- Plutôt que de tester la taille d'un entier et quitter si ça ne convient pas "à la main", utilise les outils qu'on te fournis ! Je pense notamment aux "assert" qui sont dans <cassert>. Exemple: assert(sizeof(unsigned int) == 4 && "32 bits unsigned int type needed");
- Si tu n'utilises pas d'héritage virtuelle, il est déconseillé de déclarer un destructeur virtuel. Tu vas "payer" le prix d'une classe virtuelle pour rien (même si dans ce cas précis, ce n'est pas "cher").
- Ton destructeur n'est pas nécessaire. Celui par défaut suffit (ne pas en déclarer, reviens à utiliser celui par défaut).
- Pourquoi t'embêter à faire: unsigned int K0 = strtol ("0x5a827999",0,16); quand on peut directement écrire: unsigned int K0 = 0x5a827999; ? :)
- En C++, on utilise plutôt des reinterpret_cast et des static_cast plutôt que des "C cast", qui sont à proscrire. Ex: reinterpret_cast<unsigned int*>(&bitLen);
== Au niveau du style ==
- Indente ton code ! C'est assez horrible à lire, et c'est dommage parce que c'est la première chose que l'on voit.
- Les macros se mettent en général en majuscule. Donc #ifndef _HASH.
- Une macro fonction s'écrit en majuscule.
- Une classe prend une majuscule (écriture PascalCase).
- Une fonction s'écrit en "camelCase". Par exemple, en voyant "ROTL", j'ai cru que c'était une macro...
- i = i + 4 peut s'écrire plus simplement en i += 4.
- Un header C++ porte l'extension .hh ou .hpp afin de le différencier du C.
== Au niveau de la soumission à CS ==
- C'est un sujet intéressant que tu abordes, mais il n'y a aucune explication :(. CS est un site à vocation pédagogique. Il aurait été plus que bienvenue de présenter le sha-1, son utilité, et surtout le principe de fonctionnement de ton implémentation.
- Il y a très peu de commentaire et certains commentaire ne font que paraphraser le code :(. Il y a une opportunité d'amélioration à ce niveau là. Voir comment écrire de bon commentaires ici: http://0217021.free.fr/portfolio/axel.berardino/articles/ecrire-de-bons-commentaires
- Vu que tu bosses sous Unix, joindre un Makefile aurait été bien pratique.
15 janv. 2015 à 20:46
Merci cptpingu pour des commentaires aussi precis .
Je tiens a te repondre sur certaines de tes interrogations .je suis conscient que mes reponses ne rendront pas mon code meilleur .Mais je veux expliquer dans quel esprit j'ai ecrit ce code.
Tout d'abord j'ai implementé ma fonction de hash sha1 dans une classe pour
pouvoir la completer plutard avec une fonction sha 256 ou hmac .
Pour les noms de variables j'ai repris les memes noms que sur la documentation officiel ;c'est a dire la FIPS PUB 180-3 .Je voulais que quelqu'un avec la meme doc puisse retrouver facilement les variables dans le code .
Je n'avais pas compris l'aspect pedagogique de CS .Pour moi c'etait un "coffre" dans lequel chacun y mettait sa contribution ,c'est a dire du "code source a la bonne franquette"
Je conserve tes remarques pour mieux les appliquer .
Et merci encore ...