Discussion :

[Bktero]

Bonjour,

Pour une raison que vous ne préférez pas connaitre, j'aimerais garder une liste de toutes les instances d'une classe. J'ai écrit un code comme ceci :

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
#include <algorithm>
#include <iostream>
#include <vector>
 
struct Foo {
	static std::vector<Foo*> instances;
 
	static void list() {
		std::cout << "---" << std::endl;
		for (auto& foo : instances) {
			std::cout << foo->name << std::endl;
		}
		std::cout << "---" << std::endl;
	}
 
	Foo(std::string name) :
			name(name) {
		instances.push_back(this);
	}
 
	Foo(const Foo&) = delete;
	Foo& operator=(const Foo&) = delete;
 
	~Foo() {
		auto endIt = std::remove(instances.begin(), instances.end(), this);
		instances.erase(endIt, instances.end());
	}
 
	std::string name;
};
 
std::vector<Foo*> Foo::instances;
 
int main(void) {
	{
		Foo a { "a	" }, b { "b" }, c { "c" };
		Foo::list();
		{
			Foo d("d");
			Foo::list();
		}
		Foo::list();
	}
	Foo::list();
}

Il affiche :

---
a	
b
c
---
---
a	
b
c
d
---
---
a	
b
c
---
---
---

- Quels commentaires pourriez-vous faire sur ce code ? Il n'est pas thread-safe mais ce n'est pas un problème ici.

- Quels conteneurs auraient été mieux ?

- Ma technique de suppression d'un élément du vector est-elle correcte ?

Merci d'avance !

EDIT : je rends compte que l'effacement d'un seul élément se fait très bien comme ça :

Code :

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

1
2
		std::remove(instances.begin(), instances.end(), this);
		instances.pop_back();

[Bousk]

Salut,

y'a pas beaucoup d'options pour faire une telle chose.
N'oublie pas d'aussi supprimer les constructeurs et affectation par déplacement. Mais ceci va bloquer l'utilisation de ta classe dans uen collection... Du coup tu devras surement implémenter le constructeur de déplacement, mais laisser l'affectation supprimée.
Sinon, le constructeur de déplacement ne faisant pas l'insertion dans la liste, ton destructeur va supprimer la dernière instance de ta liste à cause du pop_back.
Après la seule chose éventuellement à modifier c'est d'utiliser un set au lieu d'un vector. Mais faut vérifier si ça apporte un gain ou non. Ca va dépendre aussi du nombre d'instances que tu comptes avoir, et à quelle fréquence tu en ajoutes/supprimes et lesquelles (fin de liste ? début ? milieu ?).

[jo_link_noir]

Pour la suppression, si l'ordre n'est pas important, mettre le dernier élément à la place de celui trouvé puis faire un pop_back sera plus rapide.

Code :

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

1
2
*std::find(instances.begin(), instances.end(), this) = instances.back();
instances.pop_back();

[koala01]

Salut,

Moi, je n'ai qu'un seul commentaire à faire : tu ne respecte pas le SRP

Si tu veux garder "quelque part" une trace de toutes les instances de ta classe, tu devrais prévoir "quelque chose" dont ce serait la seule responsabilité (un FooHolder, peut-être ), et qui ne serait pas forcément accessible depuis ta structure Foo.

Cela peut nécessiter quelque aménagements, comme le fait de clairement séparer les responsabilités en quelque chose comme

• FooHolder : maintient les Foo qui sont créées (et les détruit quand elles sont inutiles)
• FooFactory : crée des Foo à la demande
• "QueqlueChose" qui fait le lien entre FooHolder et FooFactory et qui permet de créer des Foo à la demande et d'accéder aux Foo existants (interface : begin, end, find, add, remove, ...)

J'ai bien conscience que cela peut sembler quelque peu "overkill", mais si tu commence à mélanger le tout en ajoutant des fonctions statiques à ta structure uniquement pour être en mesure de dresser "de temps en temps" la liste des instances existantes, toi et ton équipe aurez tendance à "profiter" de cette opportunité pour commencer à le faiire "n'importe où" à peu près "en dépis du bon sens".

Et le résultat ne se fera pas attendre : Tu auras très rapidement de nombreux endroits où tu observeras une dépendance vis à vis de ta structure, alors qu'elle n'aurait normalement pas eu lieu d'être

[wolinn]

Il faut voir quel est l'usage. Il m'est arrivé de faire ce genre de chose, code du même type que bktero, provisoirement en debug, pour traquer des fuites de mémoire, mais ce code disparaissait ensuite, et donc il n'était pas utile de construire des structures plus élaborées (et même en C++ 11, où ce genre de méthode est en principe moins utile, je garde quand même parfois un compteur d'instances, juste un entier statique incrémenté/décrémenté, sans conserver les instances elles-mêmes, ça ne coûte pas très cher).

[koala01]

Pour du code "jetable", parce que tu as, pour un "très court instant", besoin de "plus d'informations" que ce que dont tu peux disposer en débug, je ne dis pas forcément le contraire

Le problème, c'est quand le ce code (soit disant) jetable est prévu pour être utilisé "provisoirement" (un "hack" pour faire fonctionner quelque chose).

Par ce que ce code "provisoire" aura la facheuse tendance à se transformer en code "définitivement provisoire", sur lequel s'appuiront de plus en plus de fonctionnalités au fil du temps, ce qui en rendra le retrait encore plus difficile.

C'est le typique exemple de ce qui pourrait rapidement devenir une belle "dette technique": en voulant gagner cinq ou dix (ou même, mettons 60, vu qu'il y a tout un système à mettre en place), on va perdre, au fil du temps, des jours entiers à essayer de contourner et / ou de résoudre des problèmes qui ne seront apparus que parce que tu as fait ce choix malheureux

Quand j'étais ambulancier, l'une de nos phrases fétiches était "vite, mais sans précipitation". Cela signifiait que l'on ne devait "pas trainer" à faire ce que l'on avait à faire (parce qu'on tenait parfois littéralement la vie de certaines personnes entre nos mains), mais que l'on ne devait pas se précipiter, car il suffisait que l'on trébuche (quand cela ne signifiait pas avoir un accident avec l'ambulance) pour perdre encore plus de temps que ce que l'on espérait gagner en le faisant.

Je sais très bien que certains gestionnaires financiers vont râler si on passe plus de trois minutes à faire quelque chose qui leur semble simple. Mais si les développeurs en général devraient aussi comprendre que "que va piano va sano"

[ternel]

Prends garde que des instances peuvent aussi être créées par les constructeurs de déplacement.
Supprimer la copie ne supprime pas le déplacement, c'est d'ailleurs exactement ce que font les *streams et unique_ptr.

[jo_link_noir]

Supprimer la copie ne supprime pas le déplacement

Si, à partir du moment qu'une fonction de copie existe, l'équivalent par déplacement est cachée. Il faut alors mettre =default pour la rendre accessible.