Discussion :

[loustak]

Bonjour,
j'ai un problème de conception, j'ai une classe de base qui gère les ressources défini comme suit :

Code :

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

1
2
3
4
5
class RessourceManagerBase { 
public:
	virtual ~RessourceManagerBase() { }
	virtual void construct(nlohmann::json& json) = 0;
};

Puis vient cette classe template :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
template <typename T> class RessourceManager : public RessourceManagerBase {
public:
	void construct(nlohmann::json& json) { }
 
	std::map<std::string, T*>& getList() const {
		return _object;
	}
 
private:
	std::map<std::string, T*> _object;
};

Afin d'être utilisé dans le code comme suit :

Code :

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

1
2
std::map<std::string, RessourceManagerBase*> _manager;
_manager["tile"] = new RessourceManager<Tile>();

Jusque là aucun soucis. Le problème c'est comment est-ce que je peut récupérer les objets contenus dans _object.
Je voulais écrire ça :

Code :

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

std::map<std::string, Tile*> t = _manager["tile"]->getList()

Mais vu que _manager stock des string et des RessourceManagerBase* ça ne peut pas fonctionner.
Je ne peut pas mettre ma fonction en virtuel dans la classe RessourceManagerBase puisque elle retourne un template...
Du coup je ne sais vraiment pas comment faire, merci de votre aide.

[Bousk]

Ta conception est bancale, pourquoi seule le dérivé pourrait retourner une liste ? Pourquoi la classe de base n'est pas le template, quelle est son utilité et sa plus-value ?

Perso mes resources managers ressemblent toujours à ça (raw copié/collé donc y'a du bruit désolé, et c'est du code que je trimballe depuis des années que l'on peut aisément améliorer via shared_ptr et autres joyeusetés du genre apparu depuis )

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
namespace BouSdK
{
	//!< specialize this function for each resource type needed
	template< class T >
	T* LoadResource(const std::string& _strName)
	{
		ALERT("Loader not found");
		Log::Get().Critical("Unable to load Resource : %s : No loader defined !", _strName.c_str());
		return NULL;
	}
 
	template< typename T >
	class CResourcesManager : public CSingleton< CResourcesManager<T> >
	{
		struct Holder : public CRefCpt
		{
			T* m_pRsc;
			Holder() : m_pRsc(NULL) {}
			~Holder()
			{
				SafeDelete(m_pRsc);
			}
		};
		typedef std::map<std::string, Holder*> MapRsc;
		MapRsc m_map;
		void ReleaseResource(typename MapRsc::iterator it)
		{
			it->second->DecRef();
			if (it->second->GetRef() == 0)
			{
				SafeDelete(it->second);
				m_map.erase(it);
			}
		}
		public:
			~CResourcesManager()
			{
				for (MapRsc::iterator it = m_map.begin(); it != m_map.end(); )
				{
					it->second->DecRef();
					SafeDelete(it->second);
					m_map.erase(it++);
				}
			}
			T* GetResource(const std::string& _strName)
			{
				MapRsc::iterator it = m_map.find(_strName);
				if (it != m_map.end())
				{
					it->second->IncRef();
					return it->second->m_pRsc;
				}
				T* pTRsc = LoadResource<T>(_strName);
				if (!pTRsc)
					return NULL;
				Holder* pRsc = new BOUSDK_HEAP Holder;
				pRsc->m_pRsc = pTRsc;
				m_map[_strName] = pRsc;
				return pRsc->m_pRsc;
			}
			void ReleaseResource(T*& _pRsc)
			{
				if (!_pRsc)
					return;
				for (MapRsc::iterator it = m_map.begin(); it != m_map.end(); ++it)
				{
					if (it->second->m_pRsc == _pRsc)
					{
						_pRsc = NULL;
						ReleaseResource(it);
						return;
					}
				}
			}
			void ReleaseResource(const std::string& _strName)
			{
				MapRsc::iterator it = m_map.find(_strName);
				if (it != m_map.end())
				{
					ReleaseResource(it);
				}
			}
	};
}

Que j'utilise de cette manière

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
namespace BouSdK
{
	template<>
	sf::Texture* LoadResource(const std::string& _strName)
	{
		sf::Texture* pSfTex = new BOUSDK_HEAP sf::Texture;
		if (!pSfTex->loadFromFile(_strName))
			if (!pSfTex->loadFromFile(BouSdK::Renderer::CSprite::TEXTURES_FOLDER+_strName))
			{
				Log::Get().Error("Unable to load Texture : %s", _strName.c_str());
				SafeDelete(pSfTex);
			}
		return pSfTex;
	}
}
//!< declare our resource manager for sf::Texture for faster access/usage
typedef BouSdK::CResourcesManager<sf::Texture> CSFMLTextureManager;

[loustak]

pourquoi seule le dérivé pourrait retourner une liste

Car si j'ai bien comprit ce que j'ai lu sur internet je ne peut pas écrire ceci :

Code :

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

1
2
3
4
5
6
7
class RessourceManagerBase { 
public:
	virtual ~RessourceManagerBase() { }
	virtual void construct(nlohmann::json& json) = 0;
        // Ajout
	virtual std::map<std::string, T*>& getList() const = 0; 
};

Pourquoi la classe de base n'est pas le template, quelle est son utilité et sa plus-value ?

L'idée derrière c'est de pouvoir créer une map contenant tous les managers, peut importe le type sur lequel ils portent. Comme ceci :

Code :

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

std::map<std::string, RessourceManagerBase*> _manager;

Et après l'ajout d'un manager :

Code :

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

_manager["tile"] = new RessourceManager<Tile>();

C'est vraiment quelque chose d'important dans mon code.

Encore merci de ton aide.

[JolyLoic]

Je pense que c'est une mauvaise idée de vouloir mettre tous les managers dans une map. Quand tu vas utiliser un manager de tiles, c'est pour gérer des tiles. C'est à dire qu'au point d'utilisation de ton manager, tu vas très certainement devoir savoir que l'objet que tu récupères est un tile. Dans ces circonstances, quel est l'intérêt de manipuler un manager de tiles comme s'il était un manager de n'importe quoi, et de le gérer dans une map qui mélange tous les types de managers de la terre.

Dit autrement, je pense préférable au point d'utilisation d'utiliser (par exemple avec un singleton, ce genre de manager est peut-être un des rares cas où ce pattern n'est pas mauvais, mais des alternatives existent):

Code :

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

1
2
3
std::map<std::string, Tile*> t = TileManager::instance()->getList()
// Plutôt que
std::map<std::string, Tile*> t = _manager["tile"]->getList()

Pourrais-tu nous indiquer pourquoi tu tiens tant à cette map ?

Si vraiment tu comptes persister dans ta voie, il te faudra utiliser des dynamic_cast à un moment ou à un autre pour récupérer l'information perdue.

[loustak]

Je tiens à cette map car elle met permet ensuite de créer un objet à partir de sont nom :

Code :

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

_manager[type].construct(parametres)

[foetus]

Alors essaye de faire cela avec un void* et de caster au moins avec la classe mère

Code :

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

std::map<std::string, void*> _manager;

[ternel]

Il y a bien Boost.Variant, qui peut t'aider.

[Astraya]

Bonjour,

Tu peux , dans ce cas, réaliser une reflection en C++.
http://www.gamasutra.com/view/featur...e_.php?print=1
http://www.gamasutra.com/view/news/1...inter_Cell.php
https://manu343726.github.io/2016/03...ing-types.html

[loustak]

Merci de toutes vos réponses. J'ai finalement réussi à implémenté quelque chose qui me convient.
Bon j'ai garder ma map de manager, sauf que cette fois si ils pointent vers des managers existant ailleurs.

Code :

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

1
2
std::map<std::string, RessourceManagerBase*> _manager; // Ma liste de manager
RessourceManager<Tile>* _tile; // Un manager parmis tant d'autre

Du coup à la création je fait ceci :

Code :

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

1
2
_tile = new RessourceManager<Tile>();
_manager["tile"] = _tile;

Ce qui me permet d'écrire ceci :

Code :

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

_manager.at("tile")->construct(parametre);

Exactement comme je le voulais.

Merci beaucoup de votre aide !

[koala01]

Salut,

J'arrive un peu après la tempête, mais j'ai quand même quelques réflexions à faire.

Déjà, le terme "manager" est un très mauvais choix car il signifie que tu décide de créer une classe qui ... gère "quelque chose" et que le terme gestion reprend tellement de choses que tu dis systématiquement adieu au SRP:

Tu commencera sans doute par vouloir créer et maintenir les objets (ce qui fait déjà deux responsabilités), puis, tu te dira que "si il gère les objets, il peut les afficher", puis que "s'il gère les objets, il peut les modifier", puis que "si il gère les objets, il peut ... (vas savoir quoi)"...

Le problème, c'est que tu finira par avoir une "méga classe" qui ... fait à peu près tout (sauf le café, sans doute) et qui sera donc utilisée ... à peu près n'importe où. Et c'est alors que les problèmes vont commencer à arriver. Car cette "méga classe" finira par être tellement complexe que tu te retrouveras dans une situation où le moindre changement (parfois même un correction de bug) aura de grandes chances de "tout faire pêter"!!! : tu finiras par te retrouver dans une situation de couplage excessivement fort de tes données.

Et je passe, bien sur, sur tous les problèmes qui seront liés aux tests unitaires et autres vérification de la bonne qualité de ton travail

L'idéal est donc de choisir des termes beaucoup plus précis, et de distinguer très clairement les différentes responsabilités. A la base, tu devrais sans doute avoir :

• une fabrique (factory??) qui s'occupe de créer tes objets (différents DP sont envisageables sur ce coup)
• un propriétaire (holder??) qui s'occupe de garder les objets en mémoire tant qu'ils sont utilisés (et de donner accès à ces objets à "qui le demande")
• un (ou plusieurs) utilisateur(s) / manipulateur(s) / visiteur(s) / autre(s) qui s'occuperont de... manipuler (de manière générale) les objets auxquels ils ont accès
• (qui sait) peut être un chargeur et un sauvegardeur (loader / saver, afin de disposer des deux parties du même tout) qui s'occuperont l'un de charger les données et l'autre... de faire en sorte que ces données soient disponibles pour le loader en les "sauvegardant".

En outre, je suis pour le moins frileux à l'idée d'avoir une classe de base (non template) commune à tous les "manager", car, d'ici à ce que tu veuille créer un "gestionnaire de managers", il n'y a qu'un pas que je suis sur que tu t'apprête à franchir

Or, ce genre de pratique va -- encore une fois -- générer un couplage tel que tu en arriveras très vite à une situation dans laquelle il te devient totalement impossible d'apporter le moindre changement sans risquer de "tout casser"...

[loustak]

Salut,

Tu commencera sans doute par vouloir créer et maintenir les objets (ce qui fait déjà deux responsabilités), puis, tu te dira que "si il gère les objets, il peut les afficher", puis que "s'il gère les objets, il peut les modifier", puis que "si il gère les objets, il peut ... (vas savoir quoi)"...

C'est la première fois que je fait un programme aussi gros donc je te l'admet je n'est sûrement pas autant de recul que toi pour ce genre de chose.
Ce que j’appelle manager, c'est la classe qui charge/décharge et permet d’accéder à tous les autres managers (les RessourceManager) du programme. Tous les objets à l'intérieur sont constant une fois créer, et ne sont jamais affiché, ou gérer ou quoique ce soit par cette classe. C'est juste une sorte de base de données dans laquelle j'ai créer une table ou je stock tous les objets eux même créer par les utilisateur hors du programme.

Cependant c'est vrai que le nom "manager" n'est peut être pas très adapter et que cette classe devrait être découpé en deux.

Mais bien sûr je suppose que je verrais tout cela plus en détail avec le temps...

[koala01]

Salut,


C'est la première fois que je fait un programme aussi gros donc je te l'admet je n'est sûrement pas autant de recul que toi pour ce genre de chose.
Ce que j’appelle manager, c'est la classe qui charge/décharge et permet d’accéder à tous les autres managers (les RessourceManager) du programme.

ET ca, ca fait déjà trois responsabilités, si par décharger, tu comprends "sauvegarder" (deux si c'est le terme que tu as choisi pour "détruire)

Tous les objets à l'intérieur sont constant une fois créer, et ne sont jamais affiché, ou gérer ou quoique ce soit par cette classe.

Pas pour l'instant...

Mais une donnée qui n'est jamais manipulée (quelle que soit la manière dont elle est manipulée) est -- par définition même -- une donnée inutile, et donc une donnée dangereuse, vu qu'elle risque d'interférer avec les données qui -- elles -- sont utilisées...

C'est juste une sorte de base de données dans laquelle j'ai créer une table ou je stock tous les objets eux même créer par les utilisateur hors du programme.

Mais cette "table", tu finiras bien par en faire "quelque chose"... Autrement, c'est tout le programme qui ne sert absolument à rien

Cependant c'est vrai que le nom "manager" n'est peut être pas très adapter et que cette classe devrait être découpé en deux.

Au minimum en deux (la partie "factory" et la partie "holder")...

Par la suite, il y aura tout ce qui s'occupe de la manipulation proprement dite, même si cela doit se limiter à introduire les données dans un QTableView...

Mais bien sûr je suppose que je verrais tout cela plus en détail avec le temps...

Justement, non...

Le temps est sans doute ton pire ennemi (à part toi-même s'entend) sur ce projet, car, pour l'instant, il n'y a "pas grand chose" à modifier pour assurer l'évolutivité et la maintenabilité de ton projet.

Mais, plus tu tardera à envisager les modifications qui s'avèrent nécessaires, plus le travail pour y arriver sera important, vu que la quantité d'éléments qui dépendront de ton manager n'aura cessé de croitre.

Il faut bien te dire que la complexité d'un projet n'évolue jamais que dans un seul sens : vers une complexité accrue.

Or, avec un couplage fort auquel tu risques d'arriver "simplement" parce que tu auras "décidé de ne pas décider" aujourd'hui au sujet d'une décision qui doit être prise, plus ton projet sera complexe, plus tu auras du mal à tout modifier afin d'essayer de démêler le tout et de réduire ce couplage.