Discussion :

[phi1981]

Bonjour !

Voilà je maintiens un programme assez important en C++ dont 99.9% du code est très portable (je veux dire par là qu'il ne dépend que de la librairie Standard C++, ce qui inclut la STL).
Le 0.1% non-portable concerne une toute petite classe qui permet d'écrire des données vers une sorte de périphérique.
Cette classe était définie jusqu'à présent sous Linux un peu de cette façon :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
// OutputDevice.h
 
class OutputDevice
{
public:
// .... Diverses méthodes, constructeurs & destructeur
//
      void Write (const std::string& out);
private:
   int m_fd; // Descripteur de fichier
};

Je dois réimplementer cette classe sous Windows, et ce qu'elle doit faire est tellement simple que l'api native Win32 suffit parfaitement à mes besoins.
Mon problème, c'est que pour obtenir une classe similaire je dois faire ça :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
#include <windows.h>

class OutputDevice
{
public:
// .... Diverses méthodes, constructeurs & destructeur
//
      void Write (const std::string& out);
private:
   HANDLE m_fd; // Descripteur de fichier, version Windows
};

HANDLE est un type définit par Windows.h (ou un header inclut par ce dernier), et inclure Windows.h pollue le namespace global d'un tas de trucs qui rendent le reste du code (censé être portable) pas forcément évident à maintenir. Parmis ces nuisances figurent énormément de macros comme min()/max() et un tas de noms de fonctions comme CreateFile() qui est implémenté comme une macro pour CreateFileA() et CreateFileW().

Je pourrais tout à fait utiliser un pImpl (pointeur sur implémentation) et donc avoir une autre classe qui se charge d'allouer un objet opaque contenant un HANDLE:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
class OutputDevice
{
     class Impl;

public:
// .... Diverses méthodes, constructeurs & destructeur
//
      void Write (const std::string& out);
private:
   std::unique_ptr<Impl>    m_Impl; // Pointe vers une classe opaque qui manipule mon HANDLE
};

Mais mince quoi !! ça voudrait dire qu'à chaque fois que j'instancie cette classe OutputDevice, je force une allocation mémoire pour stocker une variable HANDLE de 4 octets ???? Pfff .... C'est pas vraiment un problème pour les performances mais je trouve ça lourd.

Puis j'ai réfléchi : un rapide coup d'oeil à la définition de HANDLE m'apprend qu'il s'agit d'un typedef vers void*. Pourquoi est-ce que je ne définirais pas alors ma classe comme ça :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
class OutputDevice
{
typedef void* SystemHandle;

public:
// .... Diverses méthodes, constructeurs & destructeur
//
      void Write (const std::string& out);
private:
   SystemHandle    m_Handle; // Type spécifique au système 
};

Je ne pense pas casser quoi que ce soit de cette façon. Je n'empêche pas mon compilateur de faire de la vérification de type: lorsque, dans mon code non-portable, j'appelle une fonction comme WriteFile(), je peux lui passer un SystemHandle, puisqu'il s'agit du même type que celui auquel il s'attend. Et si ça venait à changer demain (si Microsoft définit HANDLE comme étant un int par exemple), j'aurais une erreur de compilation, mais facile à fixer.
Je suis juste surpris de pas avoir vu ça plus souvent dans le source que je lis un peu partout, alors j'aimerais bien des avis extérieurs.

Merci !!!

[koala01]

Salut,

Le pimple idiom est, à mon sens, plus adapté, car il permet de centraliser tout ce qui est réellement spécifique à une implémentation donnée.

Le problème, c'est que, si tu veux utiliser HANDLE (sous la forme d'un typedef perso ou sous sa forme "réelle"), tu vas devoir jalonner ton code de directives de compilation conditionnelles, car tu auras un void* pour l'implémentation sous windows et un int pour l'implémentation sur d'autres architectures.

Alors que, si tu utilises le pimple idiom, tout ce qui est spécifique à windows (et donc à l'utilisation de HANDLE) sera regroupé dans une classe spécifique, et il en ira de même pour tout ce qui est spécifique à une autre architecture, le tout, de manière totalement transparente pour l'utilisateur . La facilité de développement est sans doute au bout de cette méthode qui peut paraître un peu "overkill" vu que ce n'est que pour changer un int en void *

[phi1981]

Le problème, c'est que, si tu veux utiliser HANDLE (sous la forme d'un typedef perso ou sous sa forme "réelle"), tu vas devoir jalonner ton code de directives de compilation conditionnelles, car tu auras un void* pour l'implémentation sous windows et un int pour l'implémentation sur d'autres architectures.

J'arrive à implémenter ma pseudo-solution sans un seul #ifdef (j'ai jamais beaucoup aimé ces bêtes là, pourtant je suis un programmeur C à la base).
Pour ça, mon source est divisé avec un répertoire principal qui contient le source portable, et deux sous-répertoires : win32 et posix.
Chaque sous-répertoire contient sa version de "OutputDevice.h". Dans le source portable j'include <OutputDevice.h>, et c'est mon environnement de build (Visual Studio sous Windows, Scons sous Linux) qui définit le répertoire où aller chercher les includes.

Alors que, si tu utilises le pimple idiom, tout ce qui est spécifique à windows (et donc à l'utilisation de HANDLE) sera regroupé dans une classe spécifique, et il en ira de même pour tout ce qui est spécifique à une autre architecture, le tout, de manière totalement transparente pour l'utilisateur . La facilité de développement est sans doute au bout de cette méthode qui peut paraître un peu "overkill" vu que ce n'est que pour changer un int en void *

Si je dois en arriver là, alors pourquoi ne pas carrément utiliser l'héritage ?
avoir une classe de base abstraite OutputDevice, laisser chaque plateforme implémenter un OutputDeviceWin32 et un OutputDevicePosix, et avoir pour chaque plateforme une fonction factory qui me génère celui qui est approprié.
Après tout, cela n'a rien d'illogique : un OutputDeviceWin32 est un (is a) OutputDevice, donc un héritage public peut se justifier.
Ce que j'aime, c'est que si ma fonction factory est du genre :

Code :

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

1
2
 
std::unique_ptr<OutputDevice>   CreateOutputDevice();

Alors je laisse l'appelant gérer la durée de vie. Il peut même convertir le unique_ptr en shared_ptr si besoin.
Je crois même être tombé sur un article d'Herb Sutter qui proposait ça comme alternative à un pImpl...

[dragonjoker59]

un HANDLE est un typedef sur void *
T'est-il possible de passer ton membre m_fd en ptrdiff_t (ou size_t) sous Linux ?
Car si ça t'est possible, tu le fais, et tu n'inclues Windows.h que dans ton fichier .cpp, avec juste un cast sur la définition du handle et un sur son utilisation. (j'aime bien les solutions bêtes et connes)

[phi1981]

Oui je peux le faire, je peux même faire encore plus simple et définir un type abstrait, un peu comme SystemHandle dans mon exemple, qui varie suivant la plateforme, et ainsi éviter un cast.
Mais est-ce sain et raisonnable de reposer sur la connaissance du fait que HANDLE est juste un synonyme de void* ? Il me semble que je brise une abstraction établie par Microsoft.

[jblecanard]

Le problème, c'est que, si tu veux utiliser HANDLE (sous la forme d'un typedef perso ou sous sa forme "réelle"), tu vas devoir jalonner ton code de directives de compilation conditionnelles, car tu auras un void* pour l'implémentation sous windows et un int pour l'implémentation sur d'autres architectures.

Ce qu'on peut aussi faire, c'est configurer la chaîne de compilation pour choisir le bon *.cpp en fonction de la plateforme sur laquelle on compile. Ce qui donne un *.cpp Linux et un *.cpp Windows, comme ça pas de macro dégueux pour faire de la compile conditionnelle. Par contre, il faudra quand même se taper tous les cast dans le *.cpp, et bien surveiller que la taille du membre "générique" correspond bien une fois casté.

Et en effet, tu briserais une abstraction, et l'idéal reste un bon pimp bien propret.

[white_tentacle]

Si j’ai bien compris :

- à la compilation, en fonction de la plateforme, tu vas utiliser OutputDevice qui est dans le répertoire linux, ou OutputDevice qui est dans le répertoire windows
- chaque OutputDevice est spécifique à la plateforme
- le client doit pouvoir utiliser indifféremment un OutputDevice ou l’autre sans faire la différence.

Conclusion : OutputDevice est une structure opaque, du coup pimpl me semble tout à fait adapté.

Maintenant, si ce qui te gêne, c’est l’allocation dynamique que ça implique, tu peux faire de la micro-optimisation.

La seule chose que les clients ont besoin de connaître, c’est la taille de ta structure (et encore, tu peux t’arranger pour que ce ne soit pas le cas). Donc, dans ta classe, tu peux mettre un :

Code :

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

1
2
3
4
class OutputDevice {
char reserved[sizeof(void*)]; // ou sizeof(long long), ou 8, ou 12
…
};

Ensuite, dans ton cpp :

Code :

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

1
2
3
4
5
 
HANDLE& getHandle(OutputDevice& d) {
    static_assert(sizeof(HANDLE) <= sizeof(OutputDevice::reserved), "Size mismatch :*HANDLE too big");
    return reinterpret_cast<HANDLE&>(d.reserved);
}

Le static_assert te permet de t’assurer que dans tous les cas, tu as réservé assez d’espace pour stocker ton HANDLE. Côté client, c’est totalement opaque : il sait juste qu’il y a X octets réservés pour la structure, qui sont privés.

[phi1981]

Donc, dans ta classe, tu peux mettre un :

Code :

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

1
2
3
4
class OutputDevice {
char reserved[sizeof(void*)]; // ou sizeof(long long), ou 8, ou 12
…
};

L'idée est intéressante, et ça se rapproche d'une solution discutée pour Boost (http://lists.boost.org/Archives/boos.../10/171508.php) qui expose un problème similaire.
Mais si je fais ça, comme expliqué plus haut, je brise une abstraction : celle qui me permet d'ignorer que HANDLE est synonyme de void*.
Imagine si demain je porte mon soft vers une plateforme où l'API système, plutôt que d'utiliser un type primitif pour stocker un descripteur de fichier, utilise une structure de 48 octets (pour une raison justifiée). Cela voudrait dire que je réserve 48 octets pour ma classe OutputDevice, et ce pour une seule plateforme, alors que les autres (Windows et Linux) se contentent de respectivements 8 et 4 octets ?
Si je paye le prix de briser l'abstraction, je préfère faire plus propre et avoir une définition de "SystemHandle" qui varie d'une plateforme à l'autre, elle aura au moins l'avantage d'être toujours de la bonne taille, et ce sans besoin d'un static_assert().
Elle ne va pas non plus nécessiter un cast, puisque les données manipulées (void* d'un coté, void* de l'autre) sont compatibles.

[white_tentacle]

L'idée est intéressante, et ça se rapproche d'une solution discutée pour Boost (http://lists.boost.org/Archives/boos.../10/171508.php) qui expose un problème similaire.
Mais si je fais ça, comme expliqué plus haut, je brise une abstraction : celle qui me permet d'ignorer que HANDLE est synonyme de void*.
Imagine si demain je porte mon soft vers une plateforme où l'API système, plutôt que d'utiliser un type primitif pour stocker un descripteur de fichier, utilise une structure de 48 octets (pour une raison justifiée). Cela voudrait dire que je réserve 48 octets pour ma classe OutputDevice, et ce pour une seule plateforme, alors que les autres (Windows et Linux) se contentent de respectivements 8 et 4 octets ?

Pas tout à fait. En fait, l’idée, c’est de dire : je paie un pointeur, dans tous les cas, pour pouvoir faire du pimpl.

Oui, mais, dans les cas linux et windows, c’est con, la taille de mon object pimpl est inférieure ou égale à la taille du pointeur lui-même : bon, je fais un truc « sale », je stocke directement la donnée dans le pointeur, et je m’assure par un static_assert que si un jour ça ne tient plus, mon code ne compile plus. Si jamais je suis sur une plateforme où la taille du descripteur de fichier ne tient pas dans mon pointeur, là j’ai toujours la solution de faire un pimpl classique avec allocation des données privées dans le tas. Et la taille de mon OutputDevice ne change pas.

C’est bien pour ça que j’ai parlé de micro-optimisation : tu fais du pimpl, mais en réalité tu stockes la donnée privée directement dans le pointeur vers pimpl.

Si je paye le prix de briser l'abstraction, je préfère faire plus propre et avoir une définition de "SystemHandle" qui varie d'une plateforme à l'autre, elle aura au moins l'avantage d'être toujours de la bonne taille, et ce sans besoin d'un static_assert().
Elle ne va pas non plus nécessiter un cast, puisque les données manipulées (void* d'un coté, void* de l'autre) sont compatibles.

En fait, que tu écrives :

Code :

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

1
2
3
char reserved[sizeof(void*)];
void* pimpl;
SystemHandle handle; // avec typedef void* SystemHandle;

Ne change pas grand chose d’un point de vue implémentation. À toi de voir quelle solution est la plus lisible à tes yeux.

[JolyLoic]

Si je dois en arriver là, alors pourquoi ne pas carrément utiliser l'héritage ?

Parce que l'héritage est aussi peu performant, et impose à l'utilisateur de ton type de manipuler des pointeurs, alors qu'avec le pimpl, il peut sans soucis manier des objets.

[phi1981]

C’est bien pour ça que j’ai parlé de micro-optimisation : tu fais du pimpl, mais en réalité tu stockes la donnée privée directement dans le pointeur vers pimpl.

Oh ok merci je comprends mieux ce que tu voulais dire. En effet c'est intéressant de dire "je peux utiliser du pImpl, mais dans ce cas précis je ne le fais pas, parce que je me permets de briser une abstraction (que HANDLE est synonyme de void*)."

En fait, que tu écrives :

Code :

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

1
2
3
char reserved[sizeof(void*)];
void* pimpl;
SystemHandle handle; // avec typedef void* SystemHandle;

Ne change pas grand chose d’un point de vue implémentation.

Là par contre je t'avoue que je suis pas. La solution SystemHandle ne requiert pas de cast dans l'implémentation ce que je trouve pas négligeable du tout. Je peux définir SystemHandle de façon différente suivant la plateforme :

Code :

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

1
2
3
4
5
6
7
 
// Sous Linux, qui a l'avantage d'utiliser un int comme descripteur:
typedef int SystemHandle;
// Sous Windows, qui utiliser un void* déguisé en HANDLE:
typedef void* SystemHandle;
// Sous un OS imaginaire qui utilise un gros objet, je peux toujours utiliser un pImpl:
typedef std::unique_ptr<Impl> SystemHandle; // Le nom SystemHandle est peut être mal choisi, toutefois...

Aussi, je préfère utiliser un pointeur intelligent pour mon pImpl, plutôt qu'un void*. Et je me vois mal "caster" mon HANDLE vers un unique_ptr, même si en théorie ça devrait marcher.

EDIT: JolyLoic: En effet, j'avais oublié cet aspect: un pImpl ne requiert pas la virtualité, alors qu'utiliser l'héritage dans mon cas oui.

[r0d]

Quelque chose m'a certainement échappé, mais pourquoi n'utilises-tu pas fstream ou boost::filesystem?

[phi1981]

C'est vrai que je n'ai pas donné beaucoup de détails sur mon histoire. Le logiciel est embarqué dans un petit système et il émet des données de débogage sur un port série (il n'est pas exclu qu'on redirige cette sortie vers autre chose plus tard).

Si je devais recommencer le projet à partir de zéro, je chercherais évidemment du coté de boost (filesystem, iostreams ou autre) pour voir si, par hasard, y a pas une façon de gérer un port série de façon portable. Je ne crois pas que la librairie standard le permette (fstream & co).
Sous Linux, initialiser un port série ne représente rien de plus que d'appeler open() suivi de quelques appels à ioctl().
Sous Windows, ce n'est guère plus compliqué, c'est juste que les fonctions ne portent pas le même nom.
Incorporer un composant externe, fut-il aussi bien écrit que boost, simplement pour une sortie debug, alors que tout le reste du code repose simplement sur la librairie standard, c'est pas forcément évident à faire accepter.

EDIT: J'oubliais un détail, ma sortie de debug sous Windows permettra, dans certains cas, d'utiliser une console (via AllocConsole etc) pour visualiser la sortie plutôt que le port série. Ecrire sur une console sous Windows se fait via un WriteFile() sur un HANDLE, ce qui me permet d'utiliser le même code.

[koala01]

si je comprends bien ce que tu essayes de nous expliquer (qui me semble personnellement parfaitement clair), cela confirme bel et bien que tu dois en arriver à obtenir une abstraction qui te permette de représenter "n'importe quelle sortie débug" et de faire en sorte que, quelle que soit la sortie débug réellement utilisée, l'utilisateur n'ait pas à s'inquiéter de savoir "ce qui se cache sous le capot".

Cela confirme bel et bien que l'utilisation du pimple idiom, associé à une hiérarchie de types dont la classe de base représenterait cette abstraction, semble bel et bien être la "meilleure solution envisageable" .

Car, d'une manière ou d'une autre, qu'il s'agisse d'envoyer la sortie vers un port série sous linux, vers un port série sous windows ou vers un fichier (sous n'importe quel système d'exploitation ), ton problème est simplement que, au final, l'utilisateur de ta classe essaye de sérialiser des informations de débug

Le pimple idiom est, justement, destiné à permettre ce genre de gestion en évitant que l'utilisateur n'ait à s'inquiéter de la manière dont les choses se déroulent sous le capot

[phi1981]

Bon ok merci beaucoup tout le monde.
Le choix final sera surement un pImpl, mais je suis content d'avoir reçu autant de feedback.

Un détail cependant que je n'ai vraiment clarifié, c'est que la sortie de debug à utiliser est connue à la compilation. Et la future possibilité d'utiliser une console Windows plutôt qu'un port série "n'est pas visible de l'extérieur": l'appellant instancie toujours la même classe (OutputDevice), avec les mêmes paramètres et la décision d'utiliser un port série ou non dépend d'une logique interne à cette classe.

[Arzar]

C'est vrai que je n'ai pas donné beaucoup de détails sur mon histoire. Le logiciel est embarqué dans un petit système et il émet des données de débogage sur un port série (il n'est pas exclu qu'on redirige cette sortie vers autre chose plus tard).

Je travaille aussi sur ce genre de système embarqué. En jetant un coup d'oeil sur la partie de notre code source qui gère la sortie d'info de debug sur le port série, sans surprise, on trouve :

Code :

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

1
2
3
4
5
6
7
class SerialPort
{
   // fonctions open, close, write
   //...
 
   void* m_handle;
}

SerialPort.h
SerialPort_win.cpp (~ 200 lignes de code)
SerialPort_linux.cpp (~200 lignes de code)

C'est la solution la plus simple et la plus évidente.
Perso, les autres solutions présentées jusqu'ici (pimpl, héritage, ou grand dieux boost) m'ont l'air complètement overkill pour un si petit besoin.

Et si ça venait à changer demain (si Microsoft définit HANDLE comme étant un int par exemple)

En faisant exploser au passage la quasi totalité des applis windows existantes ?
C'est ce genre de raisonnement qui conduit à faire trop compliqué...

Pourquoi ne pas garder un code minimal, clair, quitte à le refactorer plus tard si un réel besoin se fait sentir ? (et je parle d'un vrai besoin, pas d'une vague sensation) Pour info, nous faisons aussi de la sortie de debug sur console, par udp et sur fichier. D'après les logs svn la classe SerialPort a subi pas mal de modif au cours des années et pourtant personne n'a ressenti un besoin suffisant pour toucher au void* du handle.

[koala01]

Je ne remets pas tes constatations en doute, très loin de là

Mais, si tu allais un tout petit peu plus loin que cela et que tu essayais de compter le nombre de #ifdef WIN32, #ifdef LINUX qui entourent un monstrueux *(int*) m_handle

Bon, il n'y aura sans doute pas énormément de #ifdef, vu que l'implémentation pour linux et celle pour windows est séparée, ce qui est déjà pas trop mal . Mais, combien de fois le transtypage de ton void* en int apparait il dans le code spécifique à linux

Quid si, un jour, vous décidiez effectivement de sauvegarder le fichier log sur le système embarqué "him self" La question ne se pose peut-être pas pour ton projet spécifique, est-ce une raison pour ne jamais se la poser du tout Après tout, les GPS sont des systèmes embarqués qui créent des fichiers logs pour tout un tas de choses (pour ne citer qu'un exemple concret)

[white_tentacle]

Mais, si tu allais un tout petit peu plus loin que cela et que tu essayais de compter le nombre de #ifdef WIN32, #ifdef LINUX qui entourent un monstrueux *(int*) m_handle

Bon, il n'y aura sans doute pas énormément de #ifdef, vu que l'implémentation pour linux et celle pour windows est séparée, ce qui est déjà pas trop mal . Mais, combien de fois le transtypage de ton void* en int apparait il dans le code spécifique à linux

Si c’est bien fait, une seule fois, dans une macro locale au cpp.

La seule chose qui m’étonne dans la réponse d’Arzar, c’est de considérer pimpl overkill. Le rapport bénéfice / coût de pimpl est tellement souvent favorable que la question chez moi est plutôt devenue « qu’est-ce qui justifie de ne pas utiliser pimpl ? » que l’inverse…

[koala01]

A vrai dire, ce sont bel et bien les quatre premiers mots de ton intervention qui me font peur (je me suis permis de les mettre en évidence )

Si c'est bien fait!!! Quatre petits mots, quatorze lettre et une apostrophe qui résument à merveille le doute qui peut habiter un développeur!. Ces quatre petits mots me font systématiquement me poser une question, à cause de ma manie de prévoir le pire : et si ce n'est pas le cas . C'est généralement à cause de cela que je commence à avoir peur