Bonjour,

J'implémente actuellement une api avec le Design Pattern Factory.

Voici la classe qui pose problème.

#pragma once
#include <memory>
#include "reseau.h"

class Profile
{
	template<class _Types>
	friend std::make_unique(_Types&&...);
	Profile(ProfileInfos&& profileInfos);
public:
	static std::unique_ptr<Profile> createProfile(ProfileInfos&& profileInfos);
};

et la définition de createProfile

std::unique_ptr<Profile> Profile::createProfile(ProfileInfos&& profileInfos)
{
	return std::make_unique<Profile>(std::move(profileInfos));
}

Sauf que j'ai une erreur me disant 3 fois que std::make_unique n'est pas une fonction.

Si j'enlève ma tentative de friend, le compilo me signale que make_unique essaye d’accéder à un membre privé, ici le constructeur.

Comment faire, déclaré friend la spécification de make_unique?

Merci d'avances pour vos réponses.

EDIT: pour l'instant j'utilise ça :

friend std::unique_ptr<Profile> std::make_unique<Profile>(ProfileInfos&&);

Mais est-ce propre ?

-
Edité par raphaeldesaintalbin 28 mars 2018 à 9:51:04

Salut, 

j'ai des doutes pour ton code car:

un std::make_unique est du type template  mais regarde le tien tu ne précise même pas le type dans ta classe amie .

un std::make_unique construit un élément qui sera enveloppé par un objet du type std::unique_ptr donc c.à.d qu'il renvoie un unique_ptr mais toi tu ne le précise pas dans ta classe amie.

Rendre public le constructeur de "move" ("Profile(Profile&&) = default;") me semble pas déconnant, moi.

On peut le déplacer mais pas le créer.

Tu peux être intéressé par cette discussion: https://abseil.io/tips/134

En gros:

1- on ne fait pas car ce n'est pas propre: tu rends ami un bout de code qui n'est pas tien -- et en plus qui pourrait être spécialisé pour aller taper dans tes invariants

2- au pire, tu as l'idiome pass-key

Dans tous les cas l'idée est que les utilisateurs ne vont pas passer par make_unique, mais par une fonction que tu contrôles.

Tu n'as aucunement besoin de déclarer la fonction make_unique comme amie de ta classe, vu que ta classe est bancale.

La raison de ton soucis est simple:
Le constructeur !!!  Pourquoi placer ce dernier en privé ?

Si le but est d'avoir une classe jamais instanciable (même pas par pointeur), ouais, vas-y, mais tu as intérêt à avoir des argument en béton quand à l'utilité du bousin.

> tu as intérêt à avoir des argument en béton quand à l'utilité du bousin

DP Factory, c'est pas assez béton ???

Salut,

bacelar a écrit:

DP Factory, c'est pas assez béton ???

A voir... Tout de la raison pour laquelle tu as recours à une fabrique

Je m'explique:

A priori, le recours à une fabrique est justifié par le fait que

1. ta classe fait partie d'une hiérarchie de classes,  et que
2. tu souhaites que les "utilisateurs" n'aient à connaitre que la classe de base.

Dans cette optique, tu t'en fous que le constructeur de ta classe soit publique ou non, vu que le type réel, le type concret des données ne sera connu que... dans la fabrique (et les éventuels visiteurs); et que, justement, si un éventuel visiteur doit spécifiquement construire une instance d'un type particulier pour une raison ou une autre, autant qu'il puisse aussi le faire

Maintenant, tu peux justifier le recours à une fabrique pour d'autres raisons... Par exemple, parce que tu veux "centraliser" la création de tes instances, et que tu ne veux surtout pas que des instances ne soient créer en "dehors du processus prescrit".

Mais, dans ce cas là, l'idiome du passe partout (traduction libre de pass-key  ) est largement suffisant pour éviter que qui que ce soit de "non autorisé" n'arrive à construire une nouvelle instance de ta  classe.  Et cela ne nécessite pas que le constructeur soit privé

Enfin, on peut envisager un dernier cas très particulier : les classes de base qui ne seraient pas instanciables en tant que telles, mais qui n'auraient aucune fonction virtuelle pure (ce qui en ferait automatiquement des classes abstraites).

Mais dans ce cas, l'instanciation de la classe de base doit passer par la construction d'une de ses classes dérivées.  Et, du coup, il ne s'agira pas de rendre le constructeur privé, mais bien de le rendre... protégé, pour que les classes dérivées (et elles seules) soient en mesure d'y faire appel.

-
Edité par raphaeldesaintalbin 29 mars 2018 à 11:30:01

raphaeldesaintalbin a écrit:

C'est lié avec ce que j'ai dit au dessus, ma classe Profile n'est utilisée et utilisable uniquement dans Wlan, donc oui elle n'est utilisable que dans un contexte précis.

Pourquoi ne pas simplement avoir Wlan comme amie pour qu'elle crée simplement une instance quand elle en a besoin ? (A moins que Profile soit dérivable ?).

Salut,

raphaeldesaintalbin, de ma part je voilais juste te dire que un std::make_unique<T>(Args&&...args) permet de construit un élément qui sera envelopper par un std::unique_ptr<T>... donc il n ' y a pas de sens que tu déclare un std::make_unique<T>... comme étant une fonction amie à une classe quelconque(classe X). puis dans ta classe  Profile tu déclare std::make_unique(_types&&...), tu sais pourquoi ton compilo te dit que que std::make_unique n'est pas une fonction c'est par ce que tu l'a déclaré comme étant une, comment: 

• tu n'a pas précisé le type d'élément que std::make_unique doit construire
• même en étant fonction, ton make_unique ne précise pas le type de retour( est ce un int? ou double ? ou encore type d'instance ?, ...)
• comme j'ai le  déjà dis il n ' y  a pas de sens de déclarer un std::make_unique comme ami car ce dernier était conçu pour éviter d'avoir l'opérator " new "(source de certain erreurs) dans un bon code C++

Pour le template: c'est simple tu n'as qu'à préciser le type de l'objet(élément) à construire entre les chevrons de ton std::make_unique.

exemple:

std::unique_ptr<Type> ptr = std::make_unique<Type>(Type...);

-
Edité par EL-jos 29 mars 2018 à 12:29:01

> Pourquoi ne pas simplement avoir Wlan comme amie pour qu'elle crée simplement une instance quand elle en a besoin ? (A moins que Profile soit dérivable ?).

Comme ca je ne pourrais pas vous le dire, je devais avoir une bonne raison...

Toujours est il que si je ne m'en souviens pas, c'est que la contrainte en question a du sauter lors d'un changement de conception.

@EL-jos C'est bien ce que j'avais compris 

> Pour le template: c'est simple tu n'as qu'à préciser le type de l'objet(élément) à construire entre les chevrons de ton std::make_unique.

exemple:

std::unique_ptr<Type> ptr = std::make_unique<Type>(Type...);

Oui, si le constructeur est public.

En attendant je me contente de 

Profile(Profile&&) = default;

qui me convient parfaitement.

Merci à tous !

Je t'approuve sur la mise en public du constructeur de "move".

Tu ne dupliqueras pas de "Profile" par erreur et donc moins de problèmes avec les handles de la librairie C sous-jacente.

Fais aussi en sorte que la classe Profile ne soit pas dans les headers publics de ta librairie.

Je me permet de remonter le sujet, quand je fais 

Profile(Profile&&) = default;

Ca ne marche toujours pas, impossible d'acceder à un membre privé encore une fois.

Pourquoi ?

bacelar a écrit:

Je t'approuve sur la mise en public du constructeur de "move"

-
Edité par Deedolith 5 avril 2018 à 13:42:15

Bonjour, et désolé de la longueur de ma réponse.

@Deedolith oui, c'est ce que j'ai fait.

@Bacelar donc il faudrait que tous les champs soient en public ? On revient donc au problème de base...

Quand à réimplémenter le constructeur de move de ma classe, je crois que je vais partir sur un idiome pass key, pourquoi faire compliqué quand on peut faire simple...

Merci !

>il faudrait que tous les champs soient en public ?

Bin non, il faut qu'ils soient "movable", rien à voir avec le fait qu'ils soient publics ou non.

C'est de constructeur de move qui devrait être public, pas les champs.

> je crois que je vais partir sur un idiome pass key

Si vous ne voulez pas d'un constructeur de move public, pourquoi pas, sinon, c'est assez inutile, je pense.

Oui mais comment les rendre movable ?

Ce code :

#pragma once
#include <memory>
#include "reseau.h"

class Profile
{
	Profile(ProfileInfos&& profileInfos);
public:
	Profile(Profile&&) = default;
	static std::unique_ptr<Profile> createProfile(ProfileInfos&& profileInfos);
};

Ne marche pas alors que le constructeur de move est public.

Tu peux nous poster ton code à jour, avec les dépendances ? (ce qui inclut la classe ProfileInfos).
Ainsi qu'un peut plus d'infos sur ce que tu désires faire (ce qui n'est pas ce que tu désire coder) ?

PS: Pourquoi ton constructeur est toujours privé ?
De plus, suite à l'appel de ce dernier et/ou de la fonction createProfile, le paramètre est potentiellement invalidé. Pas sûr que ce soit une bonne idée.

Bien sur, je me contenterais des headers pour l'instant, dites moi si vous voulez le reste du code.

Pour mes intentions, je vous explique juste apres le code.

> De plus, suite à l'appel de ce dernier et/ou de la fonction createProfile, le paramètre est potentiellement invalidé. Pas sûr que ce soit une bonne idée.

ProfileInfos est un DTO qui ne sert qu'a ça, vous allez le voir

Wlan.h (classe "princripale")

#pragma once
#include <string>
#include <vector>
#include "profile.h"
#include "Reseau.h"

class Wlan
{
	friend Profile;

	HANDLE m_handle;
	const GUID m_GUID;
	const std::string m_SSID;
	const std::string m_password;
	std::unique_ptr<Profile> m_profile;
public:
	Wlan(const NetworkConnectionInfos &connectionInfos, std::string password);
	static std::vector<NetworkInfos> Wlan::getNetworkInfos();
};

Classe Profile

#pragma once
#include <memory>
#include "reseau.h"

class Profile
{
	Profile(ProfileInfos&& profileInfos);
public:
	Profile(Profile&&) = default;
	static std::unique_ptr<Profile> createProfile(ProfileInfos&& profileInfos);
};

Structure NetworkInfos

#pragma once


struct NetworkInfos
{
	std::wstring profileName;
	std::string SSID;
	int nbBSSIDs;
	int nbPhyTypes;
	int signalQuality;
	int interfaceNumber;
	bool securityEnabled;
	bool haveProfile;
	GUID interfaceGUID;
	_DOT11_PHY_TYPE phyTypes;
	_DOT11_BSS_TYPE bssTypes;
	_DOT11_AUTH_ALGORITHM authAlgo;
	_DOT11_CIPHER_ALGORITHM cipherAlgo;
	NetworkConnectionInfos connexionInfos;
};

Structure ProfileInfos

#pragma once

struct ProfileInfos
{
	std::string SSID;
	std::string password;
	GUID GUID;
	HANDLE handle;
	LPWSTR xmlProfile;
	_DOT11_AUTH_ALGORITHM authAlgo;
	_DOT11_CIPHER_ALGORITHM cipherAlgo;
};

Structure NetworkConnectionInfos

#pragma once

struct NetworkConnectionInfos
{
	GUID interfaceGUID;
	int identificationNumber;
	std::string SSID;
};

Le principe est simple, encapsuler l'api Wlan de Win32 dans une API C++ de plus haut niveau.

Comment les classe interagissent entre elles:

L'utilisateur appelle la fonction publique getNetworkInfos de Wlan, puis utilise le reseau voulu (qui correspond à un NetworkInfo) au constructeur de Wlan.

Dans le constructeur de Wlan, Profile est construit avec differentes structures générées dans le constructeur.

Profile s'occupe de tester si un profile existe pour le réseau en question et le cas échéant, il en fait un.

Une fois le profile généré, Wlan s'occupe en interne de faire la connexion avec l'API Wlan en C.

Profile doit être masqué à l'utilisateur et surtout, il ne doit pas y avoir deux objets Profile identiques car cette classe comprend un HANDLE, et cela le détruirait, voila pourquoi je passe par std::make_unique.

Ta structure ProfileInfo contient des données de type HANDLE, LPWSTR ect .... des trucs propres à l'API C de Windows, bien souvent des pointeurs.

Je pense (à confirmer) que cette dernière doit également être déplacable.
pour l'instant, faute de constructeur et d'operateur adéquat, ce n'est pas le cas.

Peut être voir également si tu ne peux pas encapsuler les "pointeur C" dans des pointeurs intelligents.

Profile(Profile&&) = default;

Merci, mais cette solution a déjà été proposée mais malheureusement ça ne fonctionne pas.

Si tu as des pointeurs nu (et les pointeurs de l'API C Windows en font partie), il va falloir le faire à la main.

Exemple:

class foo
{
private:
    int* mPtr;
public:
    foo(foo&& in)
    {
        mPtr = in.mPtr;
        in.mPtr = nullptr;
    }
};

Ainsi, la classe déplacée pourra être detruite sans risques.

-
Edité par Deedolith 15 avril 2018 à 23:22:38

Salut 

Deedolith, j'ai jamais compris pourquoi aime-tu le faire à la main? aussi longtemps que la bibliothèque de la STL recommande d'utiliser la méthode 

std::exchange....

Nota: éclaircie moi un peu là dessus.

EL-jos a écrit:

Salut 

Deedolith, j'ai jamais compris pourquoi aime-tu le faire à la main? aussi longtemps que la bibliothèque de la STL recommande d'utiliser la méthode 

std::exchange....

Nota: éclaircie moi un peu là dessus.

Oh, une raison toute simple est que, cette fonction est "relativement nouvelle" (bien qu'elle soit "déjà vieille de quatre ans") et que la notion de déplacement n'est en définitive "qu'à peine plus vieille" vu qu'elle date de ... 2011.

Or, quatre et sept ans, c'est à la fois "très long" (surtout en informatique) et "très court" (à l'échelle humaine), et il faut toujours "un certain temps" pour arriver à mettre en place les bonnes pratiques associées à de nouveaux concepts.

D'ailleurs, tu peux le constater sur la page consacrée à std::exchange: elle n'est apparue qu'en C++14, alors que la sémantique de déplacement est apparue en C++11, mais elle sera "corrigée" en C++20 parce qu'on s'est rendu compte entre temps qu'elle pouvait (devait?) être constexpr.

De plus, il y a le fait que la politique générale concernant la sémantique de déplacement est de "laisser faire le compilateur chaque fois que possible", ce qui ne laisse finalement que très peu de cas (très particuliers, qui plus est) pour lesquels nous y ayons recours de manière explicite.  Cela incite "relativement peu" à s'inquiéter des bonnes pratiques en la matière

Ceci dit, tu as tout à fait raison de pointer cette bonne pratique, qui est peut-être tout simplement "passée au vert": l'idéal est -- effectivement -- d'utiliser std::exchange

-
Edité par koala01 16 avril 2018 à 11:51:11

Merci koala01, 

koala01 a écrit:

De plus, il y a le fait que la politique générale concernant la sémantique de déplacement est de "laisser faire le compilateur chaque fois que possible", ce qui ne laisse finalement que très peu de cas (très particuliers, qui plus est) pour lesquels nous y ayons recours de manière explicite.

perso, laissé la tâche au monsieur(compilo), je le trouve pas idéal car il déplacera l'objet quand il voudra(donc quand il jugera bon de le faire); 

par contre, si le développeur le fais seul(en disant au monsieur quoi faire et comment le faire), donc je pourrai déplacer mon objet au bon moment   . tu trouve pas ?

Nota: et surtout c'est la perf qu'on cherche tous en C++

-
Edité par EL-jos 16 avril 2018 à 12:12:13