JETCHRONO : HEADER DE CHRONOMÉTRAGE EN TOUTE SIMPLICITÉ
cptpingu
Messages postés
3837
Date d'inscription
dimanche 12 décembre 2004
Statut
Modérateur
Dernière intervention
28 mars 2023
-
16 févr. 2013 à 03:17
jetSett Messages postés 2 Date d'inscription mercredi 8 décembre 2010 Statut Membre Dernière intervention 8 décembre 2010 - 8 mars 2013 à 15:42
jetSett Messages postés 2 Date d'inscription mercredi 8 décembre 2010 Statut Membre Dernière intervention 8 décembre 2010 - 8 mars 2013 à 15:42
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/54961-jetchrono-header-de-chronometrage-en-toute-simplicite
https://codes-sources.commentcamarche.net/source/54961-jetchrono-header-de-chronometrage-en-toute-simplicite
8 mars 2013 à 15:42
7 mars 2013 à 19:34
C'est déjà beaucoup mieux. C'est commenté en doxygen, c'est propre et bien écris.
J'ai quelques remarques très mineures:
- Plutôt que "Ret r; [...]; r = fonct(arguments...);" préfère: "const Ret r = fonct(arguments...);". Déclarez une variable au moment de son utilisation présente 3 avantages: Pas d'oublie d'initialisation ou d'initialisation bancale, possiblité de mettre un "const", et enfin visibilité plus limité.
- En règle générale on nomme les fichiers .cc/.hh ou .cpp/.hpp (le .h étant pour du C).
- Les "using" dans le header c'est généralement pas terrible. Vu que c'est dans un namespace, ce n'est ici pas très grave. Mais sache que si l'utilisateur fait un "using namespace JetChrono", il héritera des using pour "system_clock", "time_point" et "duration"... (Néanmoins, il n'est pas censé faire un using namespace, mais plutôt un using simple, voir mieux, d'utiliser le préfixe).
Au sujet de l'exemple:
- Le code du tri à bulle est perfectible, mais vu que c'est un exemple, toute écriture est justifiable.
- "typedef vector::iterator iterator;" tu n'as pas l'air de t'en servir (sûrement un petit oubli).
- Pour le "tribulle", il y a copie du tableau en entier lors du passage en argument, et lors du retour. On fait généralement un tri "in place" (modification de l'argument, sans retour), qui évite deux copies. Dans ton exemple, tu forces un type de retour pour le besoin de la démonstration, ce qui est compréhensible. Dans ce cas, met tout de même "const std::vector& tab" plutôt que "std::vector tab", ce qui évite tout de même une copie.
- On évite les #define au profit de "const variable". C'est la même chose, sauf que le "const var"est typé, ce qui évite pas mal de petites erreurs. "#define TAILLE 1000" => "const unsigned int TAILLE = 1000;". À noter qu'une constate globale n'est pas une variable globale, et qu'il n'y a pas d'abus possible avec.
17 févr. 2013 à 16:01
Je suis débutant. Merci pour cette source.
16 févr. 2013 à 03:17
Quelques remarques:
Techniques:
- Evite les using namespace, voir: http://0217021.free.fr/portfolio/axel.berardino/articles/bon-usage-using-namespace
- Commentaires largement perfectible, voir: http://0217021.free.fr/portfolio/axel.berardino/articles/ecrire-de-bons-commentaires
- Dans l'exemple d'utilisation, tu fais une copie inutile. L'argument de "tribulle" devrait passer de "std::vector tab" à "const std::vector& tab"
- Il faut commenter/décommenter des lignes pour en activer les propriétés ! (Choix ms, us, etc...). Ca devrait être un paramètre de template avec une valeur par défaut.
- Un tri à bulle bien fait se fait avec une double boucle. (de i à fin - 1 avec une sous boucle de j = i + 1 à fin). L'exemple n'était pas très judicieux, puisqu'un simple std::sort aurait été préférable. Néanmoins c'est un test, donc rien de vraiment choquant sur ce point.
Conceptuelles:
>> "Vous devez compiler en c++11 (pas testé avec c++0x)" => C'est la même chose. C++0x est l'ancien nom de C++11.
- Si tu fais tu C++11, dans ce cas exploite le vraiment. Tu utilises un rand() + srand() alors qu'il y a bien mieux ! Regarde du côté des Mersenne Twisters (std::uniform_int_distribution, std::mt19937, ...)- De même, quand un type est compliqué, utilise "auto". "pair> p chronoFonct::chrono(tribulle, tab);"> "auto p = chronoFonct::chrono(tribulle, tab);". Cette remarque ci, contrairement aux autres, est facultative.
- Déclare une variable au moment de l'utiliser. Par exemple "Ret r;" pourrait être mis plus bas: "Ret r = fonct(arguments...);"- Plutôt que de déclarer une paire pour retourner quelque chose, fais-le à la volée. Ex: "p.first temps.count(); p.second r; return p;" => "return std::make_pair(temps.count(), r);"
- Déjà dit, mais un paramètre supplémentaire pour choisir le type de mesure devrait être présent (avec une valeur par défaut).
Fonctionnelles:
- "reproduction et modification pour des applications non commerciales autorisées" => Déjà dit de nombreuses fois, mais ne jamais mettre de pseudo avertissements de propriété intellectuelle, si on n'en mesure pas la portée. A moins que tu ais consulté un juriste, je doute que tu puisses mettre à exécution de quelconques poursuites. D'une manière générale: ne rien mettre. Surtout que cette section est réservée au partage de code (si l'utilisateur veut l'utiliser dans une solution commerciale, c'est son droit).
- Ce qui me gêne le plus, c'est que la source n'est pas très utile. Lorsque l'on mesure une performance, ce n'est pas souvent une fonction, mais plutôt un bloc de code arbitraire. Ta fonction fait économiser 3-4 lignes de code dans un cas bien particulier, alors qu'un cas plus général, aurait été beaucoup plus utile. Par exemple, en créant une fonction qui mesure le temps dans un scope (en se servant du RAII du C++) et qui fait du "pretty printing", ça aurait été bien plus utile.
- Très léger pour une source. Au final, si on retire les tests, et le fait que les deux fonctions sont quasis identiques, ça fait ~5 lignes de code:
template <typename Ret, typename... Arg>
std::pair chrono(Ret(*fonct)(Arg...), Arg... arguments)
{
auto debut = std::chrono::system_clock::now();
auto r = fonct(arguments...);
auto fin = system_clock::now();
auto temps = std::chrono::duration_cast<std::chrono::duration>(fin - debut);
return std::make_pair(temps.count(), r);
}