Bonsoir !

Voici mon souci :

Je possède une classe dont l'objectif va être de stocker les objets que je lui fournis puis de m'offrir certaines manipulations, notons Container

template<typename Key, typename Type>
class Container : private NonCopyable{

public:
    Container() = default;
    void add(const Key& key, Type& object){
        container.insert({key, &object});
    }
    
private:
    std::unordered_map<Key, Type*> container_;
    void update();
};

La raison pour laquelle j'utilise des pointeurs sur un type donné c'est simplement car la grande majorité des objets que je suis susceptibles de fournir sont soit non copiables soit issus de classes abstraites

Il est donc imposé ici, pour l'utilisation de ma classe Container, de forcer l'utilisateur à déclarer ses objets un à un, par exemple :

Button button1 {...};
Button button2 {...};

Container container {};
container.add("button1", button1);
container.add("button2", button2);

C'est pas forcément gênant pour des objets non copiables que je n'utilise pas en masse mais ça le devient quand je travaille avec des objets copiables

Je souhaiterais que l'utilisateur puisse faire cela également dans le cas d'objets copiables :

Container container {};

container.add("button1", Button(...)) // Appel du constructeur de la classe Button

J'ai donc simplement construit une surcharge de la méthode add() en passant le passage par référence en référence constante de sorte à créer automatiquement une copie de l'objet :

/* Dans la classe Container */

void add(const Key& key, const Type& object){
   
   
}

Maintenant le souci vient du fait que comme je stocke des pointeurs dans ma map désordonnée il est évident que je pointe sur un objet temporaire qui sera détruit à la fin de l'appel de add() et donc le pointeur en lui-même ne pointe plus sur rien ce qui entraînera de lourds soucis

Comment puis-je alors m'en sortir ?

Merci beaucoup pour votre temps, bonne soirée 

-
Edité par AmirProg 9 octobre 2020 à 19:19:52

Heu, sinon, les pointeurs intelligents, et en particulier l'unique_ptr, tu connais ?

Ca t’éviterait bien des prises de tête inutiles et ces pointeurs nus tout pourri.

Bonsoir, alors oui je connais mais le souci vient du fait que std::make_unique<> crée une copie d'un objet il me semble, j'ai bouquiné les pointeurs intelligents sans trouver de solutions à mon souci 

Pour être plus précis :

Si je remplace le pointeur nu par

std::unordered_map<Key, std::unique_ptr<Type>> container_;

Il ne me sera plus possible de procéder ainsi :

void add(const Key& key, Type& resource){

    container_.insert({key, &resource});
}

Il est fondamental que je puisse conserver cette méthode en passage par référence

Edit : Enfin je veux dire que je dois tout de même pouvoir ajouter des types non copiables à mon container et pas seulement des types copiables

Je souhaiterais éviter si possible de forcer l'utilisateur de mes classes à utiliser des pointeurs

SECOND EDIT :

 J'ai retiré tous les pointeurs nus et je les ai remplacés par des reference_wrapper (donc des références quoi) et quelques unique_ptr quand je le pouvais. Mon souci demeure mais au moins c'est déjà plus propre, même si cela force l'utilisateur à directement spécifier et instancier les objets nécessaires directement et m'empêche de déclarer des constructeurs par défaut...

Cependant le souci demeure avec une classe de cette forme :

template<typename Key, typename Type>
class Container : private NonCopyable{
 
public:
    Container() = delete;
    void add(const Key& key, Type& object){
        container.insert({key, object});
    }
     
private:
    std::unordered_map<Key, std::reference_wrapper<T>> container_;
    void update();
};

J'aimerais beaucoup pouvoir appeler ma méthode add mais sans nécessairement instancier les objets un à un initialement, directement au sein de la méthode ce serait préférable...

Merci pour votre réponse qui m'a aidé à épurer un peu mon code.

-
Edité par AmirProg 10 octobre 2020 à 0:26:41

La question est: qui doit être responsable du stockage de tes éléments?

Il faut être carré à ce sujet.
- ta classe toute seule -> unique_ptr
- ta classe en garde partagée -> shared_ptr,  c'est pas terrible côté design, mais... bon il arrive que cela soit ce que l'on veuille
- quelque chose externe qui garantisse que l'adresse où l'objet vit vivra plus longtemps que ton conteneur, alors en interne j'utiliserai des pointeur brut (std::reference_wrapper<> et autre mort-nés std::observer_ptr<> me paraissent compliquer plus la tâche qu'autre chose), par contre, pour passer l'objet à ton conteneur, je le ferai par référence -- c'est le design de ton exemple.

Si par contre tu veux un truc générique qui puisse faire l'un ou l'autre. Bon, c'est possible. C'est le sujet des Politiques (Policies) du vieux /C++ Modern Design/ d'Andrei Alexandrescu. Excellent ouvrage.

Ah, et il va de soit que faire l'un ou l'autre de manière dynamique, c'est la pire erreur qui puisse être commise -- et oui, en prenant soit une  référence, soit un rvalue-référence, il serait possible de savoir mais après il faudrait stocker un onwned flag, et une sorte d'union pour stocker un T*, ou un T que l'on vient de déplacer. Ca va être horriblement complexe. J'ai déjà vu, et commis(!), de multiples designs de la sorte (et encore ce n'était que T* buffer + bool owned). C'est de l'over-engineering, qui ne sert à rien en pratique, complique la maintenance, et qui n'est pas terrible côté perfs.
Aujourd'hui j'en suis arrivé à la conclusion qu'il faut parfois forcer la main à l'utilisateur en lui disant: "c'est ça mon contrat, je reste simple et efficace, pas de cas particuliers, par de questions tordues dans ta tête, moins de risques de bugs".

Quant à std::make_unique, cela ne duplique rien. Cela crée. `std::make_unique<T>(bli, bla, blu)`, c'est juste équivalent à `new T(bli, bla, blu)`. Il n'y a pas de copie.

-
Edité par lmghs 10 octobre 2020 à 0:45:54

Merci beaucoup lmghs pour cette réponse extrêmement complète et qui répond à toutes mes interrogations.

Surtout quant à l'utilisation de unique_ptr qui me paraissait un peu sombre mais je pense maintenant y voir beaucoup plus clair sur la prise en charge des données.

Tu as raison sur le fait qu'il faille forcer parfois la main à l'utilisateur, j'ai décidé finalement de ne garder que la méthode add par référence, je n'ai pas très bien compris comment se passait cette prise en charge dynamique pour le coup je l'avoue j'essaierai de m'y pencher plus tard.

J'aimerais apprendre à programmer très proprement en C++, j'ai déjà un livre d'un intervenant régulier (Koala je crois) que je dévore, c'est pas facile au début de penser de la bonne manière mais c'est un plaisir de savoir qu'on fait bien les choses. (Enfin je me rends surtout compte au début que je fais les choses très mal justement, mais bon c'est le début on va dire )

= > Me conseilles-tu ce livre dont tu parles ?

-
Edité par AmirProg 10 octobre 2020 à 1:10:53

> Tu as raison sur le fait qu'il faille forcer parfois la main à l'utilisateur, j'ai décidé finalement de ne garder que la méthode add par référence, je n'ai pas très bien compris comment se passait cette prise en charge dynamique pour le coup je l'avoue j'essaierai de m'y pencher plus tard.

Personnellement, si l'objectif ce sont les classes à sémantique d'entité, j'irai encore plus loin et forcerait la main pour avoir des unique_ptr. Je tends à me méfier des choses qui ne survivent pas, car cela peut vite va finir en dangling références. Ex:

struct UneClasse {
    void f() {
        B b1 {.....};
        B b2 {.....};
        B b3 {.....};

        m_cont.add(b1);
        m_cont.add(b2);
        m_cont.add(b3);

    } // ici b1, b2, b3 sont détruits, mais pas les références... => ka-boom

    Container<Key, B> m_cont;
};

Avec un main forcée:

template<typename Key, typename Type>
class Container
{
public:
    Container() = default;
    Containter(Container const&) = delete; // pas de cpy OK
    Containter& operator=(Container const&) = delete; // pas de cpy OK

    Containter(Container &&) = default; // aucune raison de ne pas avoir le déplacement
    Containter& operator=(Container &&) = default; // aucune raison de ne pas avoir le déplacement
    ~Container() = default; // règle du 0/5 (ou variante du "tout ou rien") => on précise
    // on aurait pu ne rien mettre car tout est implicite grâce à l'unique_ptr.

    void add(Key const & key, std::unique_ptr<Type> p){
        container_.emplace(key, move(p));
        // juste pour montrer une alternative, 
        // je ne suis pas sûr qu'elle soit plus efficace
    }

    template <typename... Params> // NB: je ne me souviens jamais où vont les '...'
    void emplace_add(Key const& key, Params&&... p)
    {
        containter_.emplace( // pareil avec insert + {}
            key, 
            std::make_unique<Type>(std::forward<Params>(p)...));
    }
     
private:
    std::unordered_map<Key, std::unique_ptr<Type>> container_;
    void update();
};

> J'ai déjà un livre d'un intervenant régulier (Koala je crois)

Le livre de Philippe? Ah ah. Je le connais. Clique sur le premier lien de ma signature.

> J'aimerais apprendre à programmer très proprement en C++

Franchement en deux fils où j'ai repéré ton pseudo (depuis pas longtemps), tu m'as épaté par tes messages. Tu es sur la bonne voix.

> = > Me conseilles-tu ce livre dont tu parles ?

*huuummmm* Aujourd'hui, en 2020, je ne peux plus décemment le faire. C'est un livre très intéressant sur le sujet de la métaprogrammation template. Et toute la partie sur les /politiques/ est extrêmement enrichissante, dans ce domaine. Alp avait écrit un article qui présentait la technique sur dpvz il y a un paquet de temps: https://alp.developpez.com/tutoriels/traitspolicies/. Andrei Alexandrescu est donc allé très loin sur le sujet avec en démonstration une classe de pointeur intelligent où l'on pouvait choisir la politique de stockage, d'erreur, de comptage, etc. Dans tous les cas, ce n'est pas fait pour mélanger comme ce que tu cherchais à faire au départ. Selon ton budget, ton temps et les sujets que tu veux approfondir, il y aura plus intéressant en 2020 comme ouvrage.

Merci encore une fois infiniment.  

lmghs a écrit:

> Personnellement, si l'objectif ce sont les classes à sémantique d'entité, j'irai encore plus loin et forcerait la main pour avoir des unique_ptr. Je tends à      me méfier des choses qui ne survivent pas, car cela peut vite va finir en dangling références.

C'est tout à fait exact je n'y ai pas pensé, ce problème ne se posera effectivement pas avec une bonne utilisation de pointeurs intelligents

Je vais étudier ton code un peu plus en détail je ne me suis encore que très peu penché sur les notions de mouvements (ce qui est dommage), c'est le moment de s'y intéresser

> Le livre de Philippe? Ah ah. Je le connais. Clique sur le premier lien de ma signature. 

Le monde est petit ! Livre très enrichissant en tout cas, bravo à vous deux c'est passionnant  (et je mets le blog en favori !)

Je compte sur toi et les autres intervenants réguliers pour prévenir le monde si des livres sont des incontournables en 2020 ! (surtout que le lien sur les bons livres de ta signature est mort )

Merci encore, j'ai beaucoup appris

Je passe ce sujet en résolu

-
Edité par AmirProg 10 octobre 2020 à 10:55:04

Bonjour,

On peut avoir un Container qui contient des objets dont il est responsable ou pas. Pour cela, on a la possibilité d'utiliser des unique_ptr qui ne sont pas systématiquement responsables.

template<typename Key,typename Type>
 class Container : private NonCopyable {
	struct Deleter {
		Deleter(bool  needDestroy = true) : needDestroy{needDestroy} {}
		void  operator()(Type*  object) { if ( needDestroy ) delete object; }
	private:
		bool  needDestroy;
	};
 public:
	void  addObject( const Key&  key, Type&  object ) {  // par ref non responsable
		auto  ptr = std::unique_ptr<Type,Deleter>(&object, Deleter{false});
		container_.emplace(key, std::move(ptr));
	}

	template<typename T=Type,typename ...Args>
	 void  addEmplace(const Key&  key, Args&&...  args) { // responsable d'une copie
		auto  ptr = std::unique_ptr<T,Deleter>(
				new T(std::forward<Args>(args)...), Deleter{true});
		container_.emplace(key, std::move(ptr));
	 }
 private:
	std::unordered_map<Key,std::unique_ptr<Type,Deleter>>  container_;
 };

Cela déroge avec l'idée de forcer l'appelant à tout gérer comme le conseille lmvhs. Cela peut alléger l'utilisation et pour limiter le risque j'ai utilisé 2 noms de fonctions distincts pour le add. Mais je plusoie sur le fait qu'avoir un Container qui assume 2 cas de responsabilités est dangereux.

> surtout que le lien sur les bons livres de ta signature est mort

Il faut être connecté avec un compte google/gmail visiblement pour voir cette liste qui n'est pas loin d’être périmée aujourd'hui.

> Mais je plussoie sur le fait qu'avoir un Container qui assume 2 cas de responsabilités est dangereux.

C'est clair que je ne suis vraiment pas fan de cette approche dynamique où dans un même conteneur on peut avoir des trucs à nous et d'autres pas. En tant que développeur utilisateur, on va systématiquement perdre du temps à lire la doc pour etre sur que cela fait bien ce que l'on espère, parfois le code, car notre expérience nous a appris à être hyper méfiant. Et en cas de bug ou de besoin d'optimiser, ce bout fera toujours parti des coupables sur lesquels on reviendra car décidément, on ne peut pas avoir confiance.

On rejoint un peu le SRP: une seule responsabilité: on possède, ou on ne possède pas. Point. Pas de questions inutiles. Pas de pirouettes.

Ce que je tolère plus facilement en revanche, c'est un objet qui va avoir une politique qui indique s'il agit comme une vue ou comme un responsable. Cf un vieil exemple ici de classe avec politique: http://hermitte.free.fr/Info/C++/libs/luc_lib/circular_list.hpp Ainsi c'est à l'écriture du code et à la compilation que l'on sait comment le conteneur fonctionne.

-
Edité par lmghs 10 octobre 2020 à 15:18:56