28 sept. 2006

Il préfère les templates...

Et quelque part, il a raison - les templates offrent un grand nombre d'avantages, et notamment l'option d'utiliser des techniques de méta programmation en C++. Pour ceux d'entre vous qui n'ont que des connaissances modestes en C++, je m'étendrais davantage sur ce sujet dans un billet ultérieur.

Même si il a raison dans un sens, notre programmeur se laisse aussi parfois prendre à son propre jeu - et le voilà qui implémente des solutions logicielles complexes dont le but est de résoudre un problème simple (pour ceux qui en doute, pour une fois je ne parle pas de moi).

Au fond de moi, je pressens que ce genre de comportement n'est pas isolé. Histoire d'enfoncer le clou et de lancer une sorte de croisade contre ce problème, voici le premier billet d'une série qui s'annonce longue. Examinons ensemble le suspect (le code a été légèrement modifié, pour des raisons assez évidente).

template <unsigned mask> class lowest_bit 
{
public:
   enum { value = lowest_bit< (mask<<1) >::value - 1 };
};
template <> class lowest_bit<0x00000000> { public: enum { value = 8*sizeof(unsigned) }; };
template <unsigned mask> class highest_bit { public: enum { value = highest_bit< (mask>>1) >::value + 1 }; };
template <> class highest_bit<0x00000000> { public: enum { value = -1 }; };
template <unsigned mask_value> class bitfield_constants { public: enum { lowest = lowest_bit<defmask>::value, highest = highest_bit<defmask>::value, left_shift = 8*sizeof(unsigned) - highest - 1, right_shift = left_shift + lowest, mask = ~0x0u << left_shift >> right_shift << lowest; }; };
template <typename int_type, unsigned mask_value> class bitfield_extractor { private: // protect creation and destruction bitfield_extractor() { } bitfield_extractor(const bitfield_extractor&) { } ~bitfield_extractor() { } public: typedef int_type value_type;
static value_type get(const value_type& val) { unsigned tmp_val_1 = static_cast<unsigned> (val << bitfield_constants<mask_value>::left_shift);
unsigned tmp_val_2 = static_cast<unsigned> (tmp_val_1 >> bitfield_constants<mask_value>::right_shift);
return tmp_val_2; }
static value_type unmask(const value_type& val) { return val & (~bitfield_constants<mask_value>::mask); }
static value_type extract(value_type &val) { value_type retval = get(val); val = unmask(val); return retval; }
static value_type merge(const value_type& val, const value_type& subval) { return (unmask(val) | get(subval)); }
static inline void set(value_type & val) { val |= mask_value; }
static bool is_notset(const value_type& val) { return 0 == (val & mask_value); } };

Ce sont là quelques lignes de code qui méritent qu'on s'y attardent - premièrement du point de vue de la syntaxe, puis ensuite au niveau de l'utilisation.

Commençons par le début : les classes lowest_bit<> et highest_bit<>. Ces classes sont des templates récursifs (une technique de méta programmation), avec une spécialisation finale qui empêche la récursion infinie. Elles marchent de manière simple : l'instanciation de lowest_bit<0x4> instancie lowest_bit<0x2> qui instancie lowest_bit<0x1> qui instancie lowest_bit<0x0> (qui est spécialisé, donc la valeur de lowest_bit<0>::value est connue au moment de la compilation). En dépilant ces instanciations, le compilateur est capable de donner à lowest_bit<0x1>::value une valeur connue, et ainsi de suite jusqu'à donner une valeur à lowest_bit<0x4>::value. Concrètement, tout ce travail est effectué par le compilateur, et lowest_bit<0x4>::value devient l'équivalent d'une constante symbolique.

Fait intéressant, ce code présuppose que les entiers sont de la taille d'un unsigned - alors que le code qui en est dérivé essaie de s'abstraire de cette limite. Mais là n'est pas le problème alors continuons.

Suite ensuite la déclaration de bitfield_constants<>, qui permet d'associer à un masque (mask_value) certaines caractéristiques. Encore une fois, du à l'utilisation des templates, toutes ces caractéristiques chiffrées sont des constantes connues au moment de la compilation. On remarque tout de même le code extrêmement complexe qui permet de calculer bitfield_constants<>::mask - complexe au point qu'on ne sait plus si cette valeur est égale à mask_value ou non (en fait, elle n'est égale que si tous les bits de value_mask se suivent. Pour donner un exemple, b01010 sera ainsi transformé en b01110).

Enfin, la classe bitfield_extractor<> elle même, qui permet l'extraction d'un certain nombre de bits dans une valeur entière.

A part de faibles erreurs d'architecture (is_notset() devrait être is_set(), l'indépendance de bitfield_extractor<> par rapport au type entier est toute relative, le choix de mettre toutes ces fonctions dans une classe est discutable, etc.), ce code est principalement correct. Un exemple simple de méta programmation, correctement lisible (même si j'ai enlevé les commentaires). Jusque là, me direz vous, ou est la faute ?

Et bien elle se trouve dans l'utilisation qui en est faite. Tout ce beau code n'est utilisé que pour instancier quelques types de classes, dont les noms sont assez évocateurs :

  • region_type_extractor
  • region_left_extractor
  • region_right_extractor
  • pca_seq_count_extractor
  • pca_feature_flag_extractor
  • search_rec_type_extractor
  • search_rec_delta_extractor

Et ainsi de suite. Tous les region_xxx travaillent sur une valeur nommée "region" dans un enregistrement de base de donnée. Idem pour les pca_xxx et pour les search_xxx. Petit à petit, un motif se dessine.

Ainsi qu'une autre solution, beaucoup plus simple, ne faisant pas appel aux templates, et ne nécessitant pas de connaissances approfondies en C++ pour comprendre comment le code fonctionne (a vrai dire, un programmeur C sera à même de comprendre). Un exemple :

struct pca_record
{
   unsigned short seq_count: 15;
   unsigned short feature_flag: 1;
};

(Mise à jour du 23/08/2013 : le code précédent utilisait le type short et non pas unsigned short. La distinction peut ne pas sembler très importante, mais elle l'est quand même (voir à ce propos le commentaire de Will ci-dessous).

Un champs de bits. Et voilà. "Méta programmation" instantanée (c'est au compilateur d'écrire le code pour lire les valeurs de seq_count et feature_flag. J'admet, j'exagère un peu quand je parle de méta programmation).

YAGNI est souvent associée à une autre pratique liée aux méthodologies agiles : il faut toujours chercher à faire la chose la plus simple qui réponde au besoin[1]. Visiblement, la chose la plus simple à faire ici aurait été d'utiliser une construction désuète - et non pas des techniques de méta programmation par templates.

D'un autre coté, cette dernière solution est pourtant bien meilleure pour l'ego. Et au diable l'avarice !

Note

[1] "Do The Simplest Thing That Can Possibly Work", ou DTSTTCPW

Commentaires

1. Le lundi, octobre 2 2006, 19:39 par Victor Nicollet

DTSTTCPW est une bonne approche à grande échelle, lorsqu'on établit quelle sera l'architecture du système et quels seront les principales approches techniques utilisées. Mais à petite échelle, je la trouve au contraire assez nuisible, pour deux raisons.

La première, c'est la viscosité (retour sur ton précédent article), dont la définition même est que DTSTTCPW produit du code incorrect, difficile à entretenir et plus généralement inacceptable.

La seconde, c'est le problème de la définition du "Work". Le contrat d'une application est plutôt simple: donner la bonne sortie pour chaque entrée sous des contraintes de temps et d'espace. C'est donc un principe facile à cognitiver à l'échelle macroscopique. Mais au niveau du code, le contrat est plus complexe, puisqu'il faut identifier à quels endroits dissimuler les informations d'implémentation (par exemple, le fait que des bit-fields sont utilisés), et à quels endroits on peut les voir. L'exemple extrême est celui d'un nombre magique: il est plus simple de l'écrire dans le code que de le transformer en constante nommée.

En bref, à petite échelle, il faut à la fois respecter le contrat (make the code work) et prendre soin du programmeur qui devra relire et modifier le code (make the code maintainable). DTSTTCPW me semble dommageable dans ce second cas. A moins de lui adjoindre des restrictions, des redéfinitions et des précisions, auquel cas il perd ce qui fait sa force: décrire en un concept court et simple une bonne pratique de développement.

Pour le problème évoqué ci-dessus, je commencerais par des fonctions d'extraction des données, et je les implanterais (secrètement) comme lisant des bit fields dans une structure comme celle que tu donnes.

2. Le vendredi, octobre 6 2006, 14:47 par Emmanuel Deloget

Bonjour Victor,

Pour ce qui est de DTSTTCPW, ça ne reste qu'une "bonne" pratique - en aucun cas un ordre. De plus, comme toi je pense que "le plus simple" est trop peu souvent "le mieux". Ceci-dit, il faut quand même ne pas aller trop loin dans la recherche de la "meilleure" solution, sans quoi on aboutit, comme dans mon exemple, à l'utilisation de techniques de méta-programmation (qui ne rendent pas forcément le code plus lisible, bien au contraire) pour effectuer des tâches qui sont somme toutes relativement simples (et très peu nombreuses).

Ceci dit, pour l'exemple que j'ai donné ci-dessus, j'aurais moi aussi tendance à encapsuler l'accès des données du bit-field dans des fonctions.

3. Le jeudi, juin 28 2007, 16:09 par Emmanuel Deloget

J'en rajoute une couche sur DTSTTCPW - avec ce lien vers la page correspondante sur c2.com: DoTheSimplestThingThatCouldPossiblyWork

Cette page résume une bonne partie des dicussions qui peuvent intervenir lorsqu'on évoque cette bonne pratique.

4. Le mercredi, août 21 2013, 11:11 par Will

Utilisez unsigned short, pas short (i.e. signed short) pour les bit fields s.v.p.

5. Le jeudi, août 22 2013, 14:21 par Emmanuel Deloget

Oh, c'est une bonne idée. En plus, c'est un point important du propos. Et j'aime les ordres venant de personnes que je n'identifie pas.

Allez, je vais le faire (mais pas maintenant - il me faut le temps de digérer l'étendue de votre sagesse).

6. Le jeudi, août 22 2013, 19:13 par Will

Plus précisément, short feature_flag: 1; est problématique.

D'abord, short est normalement équivalent à signed short, mais C++ regorge de cas particuliers (beaucoup hérités de C), et dans le cas particulier d'un champ de bits, le caractère signed ou unsigned dépend de l'implémentation, ce qui rend le code non portable. Donc le code devrait dire explicitement soit signed short feature_flag: 1; soit unsigned short feature_flag: 1;.

Ensuite, si c'est signed (ça l'est sur mon PC), alors l'unique bit de feature_flag sera interprété comme bit de parité (pragmatiquement, sur une machine qui utilise le complément à 2, feature_flag pourra valoir 0 ou -1 (pas 1), et essayer de lui assigner 1 provoquera un signed overflow, avec un comportement indéfini).

Quelqu'un pourrait quand même vouloir utiliser des champs de bits signed comme des entiers (signés) prenant moins de mémoire (par exemple, deux entiers dont les valeurs autorisées sont entre -127 et 127 inclus peuvent être représentés chacun par un signed short : 8 pour une taille totale de 16 bits (minimum) au lieu des 32 bits (minimum) que prendraient deux signed short). Mais ce genre de micro-optimisation est souvent prématuré (et de plus les opérations peuvent être plus coûteuses en temps d'exécution). (Et feature_flag, vu son nom, ne rentre pas dans ce cas.)

Plus souvent, un champ de bits est utilisé comme un simple ensemble de "flags" ou drapeaux. Le bit de poids le plus fort peut être 0 ou 1 (comme tout autre bit), et cela n'a alors pas grand sens de l'interpréter comme un signe pour le type entier servant de stockage. D'où la recommandation : pour un champ de bits utilisé comme tel, le type de stockage utilisé devrait toujours être explicitement unsigned (non-signé).

Et c'est vrai que ce n'est pas vraiment "un point important du propos", mais autant de pas donner de mauvais exemple aux débutants :) (ce n'est pas un ordre mais une requête et une forte recommandation, et je suis très lent à écrire d'où la taille très courte de mon commentaire précédent).

7. Le vendredi, août 23 2013, 11:11 par Emmanuel Deloget

Je suis d'accord sur la conclusion :) (je dirais aussi que pour écrire vite, il faut écrire beaucoup, et avoir un bon clavier avec des touches rapides).

Je vais mettre à jour l'article avec un lien vers ce commentaire.

Merci !

Ajouter un commentaire

Les commentaires peuvent être formatés en utilisant une syntaxe wiki simplifiée.

Fil des commentaires de ce billet