Comment erase un std::unique_ptr d'un vecteur en fournissant son pointeur nu ?

Invité · 09/01/2019, 17h13

Salut!
J'ai un std::vector de std::unique_ptr et je souhaiterai effacer un std::unique_ptr de mon std::vector avec la méthode erase et donc faire quelque chose comme ceci :

Code cpp :

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

1
2
3
4
5
6
7
8
9
10
 
void removeChild (LightComponent* child) {
                for (auto it = children.begin(); it != children.end();) {
                    if (it->get() == child) {
                        it = children.erase(child);
                    } else {
                        it++;
                    }
                }
            }

Mais je ne sais pas du tout comment m'y prendre, j'ai essayé de mettre mon pointeur nu dans un std;;unique_ptr mais ça ne compile pas et en plus le pointeur ne peut avoir qu'un seul propriétaire donc je ne peut pas l'affecter à deux std::unique_ptr.

**r0d** · 09/01/2019, 17h37

Hello,

tout d'abord, tu ne peux pas supprimer un élément de ton vector dans la boucle for qui itère sur tout le vector. On peut le faire avec certains conteneurs (qui en fait sont des arbres), comme le set par exemple, mais pas avec un vector.
Il faut utiliser le erase - remove idiom.

Ensuite, tu va avoir un problème pour comparer ton pointeur nu avec ton unique_ptr. Par définition, un unique_ptr n'a qu'une référence, or quand on compare directement des pointeurs, on compare leurs adresses (donc leur références). On ne peut donc pas comparer deux choses s'il n'en existe qu'une. Je suis pas sur d'être clair ^^

Mais en gros, de ce que je comprends de ton problème, il va te falloir une façon de comparer tes pointeurs.

J'ai écria un bout de code qui fournit des solutions potentielles pour ce que je viens d'expliquer. J'espère que ça pourra t'aider:

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
#include <iostream>
#include <vector>
#include <memory>
#include <algorithm>
 
struct Foo
{
	Foo(int num = 5) : m_num(num) {}
	int m_num = 5;
};
 
bool compare(const std::unique_ptr<Foo> & left, Foo* right)
{
	return left->m_num == right->m_num;
}
 
int main()
{
	std::unique_ptr<Foo> foo1 = std::make_unique<Foo>();
	Foo* foo2 = new Foo();
	bool b = compare(foo1, foo2); // b vaut true ici
 
	foo2->m_num = 3;
	b = compare(foo1, foo2); // b vaut false ici
 
	std::vector<std::unique_ptr<Foo>> v;
	v.push_back(std::make_unique<Foo>(3));
	v.push_back(std::make_unique<Foo>(5));
 
	struct CompareFonctor
	{
		CompareFonctor(Foo* foo) : m_foo(foo) {}
		bool operator()(const std::unique_ptr<Foo> & foo) const { return compare(foo, m_foo); }
	private: Foo* m_foo;
	};
	v.erase(std::remove_if(v.begin(), v.end(), CompareFonctor(new Foo(3))), v.end());
}

Invité · 09/01/2019, 17h46

Salut, j'ai essayé la solution trouvée sur ce lien : https://stackoverflow.com/questions/...-a-raw-pointer

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
void removeChild (LightComponent* child) {
                std::vector<LightComponent*> toRemove;
                toRemove.push_back(child);
                children.erase(std::remove_if(children.begin(),
                                              children.end(),
                                              [&](std::unique_ptr<LightComponent*> const &p) {
                                                return std::find(toRemove.cbegin(), toRemove.cend(), p.get()) != toRemove.end();
                                              }),
                                              children.end()
                );
            }

Mais ça me donne une erreur de compilation :

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
 
||=== Build: Debug in ODFAEG (compiler: GNU GCC Compiler) ===|
||WARNING: Can't read file's timestamp: /home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Audio/ALCheck.cpp|
||WARNING: Can't read file's timestamp: /home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Audio/AudioDevice.cpp|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../Math/../Core/archive.h||In function ‘std::ostream& odfaeg::core::operator<<(std::ostream&, odfaeg::core::OTextArchive&)’:|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../Math/../Core/archive.h|614|warning: no return statement in function returning non-void [-Wreturn-type]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../Math/../Core/archive.h||In function ‘std::istream& odfaeg::core::operator>>(std::istream&, odfaeg::core::ITextArchive&)’:|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../Math/../Core/archive.h|1363|warning: no return statement in function returning non-void [-Wreturn-type]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../Physics/../Math/computer.h|417|warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../Physics/../Math/computer.h|435|warning: comparison between signed and unsigned integer expressions [-Wsign-compare]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/component.h||In constructor ‘odfaeg::graphic::Component::Component(odfaeg::graphic::RenderWindow&, odfaeg::math::Vec3f, odfaeg::math::Vec3f, odfaeg::math::Vec3f, unsigned int)’:|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/component.h|95|warning: ‘odfaeg::graphic::Component::priority’ will be initialized after [-Wreorder]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/component.h|89|warning:   ‘odfaeg::graphic::RenderWindow& odfaeg::graphic::Component::window’ [-Wreorder]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/component.h|10|warning:   when initialized here [-Wreorder]|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Network/aes.h||In member function ‘void odfaeg::network::AES_ENC::Xor(char*, const char*)’:|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Network/aes.h|181|warning: comparison between signed and unsigned integer expressions [-Wsign-compare]|
/usr/include/c++/4.9/bits/predefined_ops.h||In instantiation of ‘bool __gnu_cxx::__ops::_Iter_pred<_Predicate>::operator()(_Iterator) [with _Iterator = __gnu_cxx::__normal_iterator<std::unique_ptr<odfaeg::graphic::LightComponent>*, std::vector<std::unique_ptr<odfaeg::graphic::LightComponent> > >; _Predicate = odfaeg::graphic::LightComponent::removeChild(odfaeg::graphic::LightComponent*)::<lambda(const std::unique_ptr<odfaeg::graphic::LightComponent*>&)>]’:|
/usr/include/c++/4.9/bits/stl_algo.h|866|required from ‘_ForwardIterator std::__remove_if(_ForwardIterator, _ForwardIterator, _Predicate) [with _ForwardIterator = __gnu_cxx::__normal_iterator<std::unique_ptr<odfaeg::graphic::LightComponent>*, std::vector<std::unique_ptr<odfaeg::graphic::LightComponent> > >; _Predicate = __gnu_cxx::__ops::_Iter_pred<odfaeg::graphic::LightComponent::removeChild(odfaeg::graphic::LightComponent*)::<lambda(const std::unique_ptr<odfaeg::graphic::LightComponent*>&)> >]’|
/usr/include/c++/4.9/bits/stl_algo.h|937|required from ‘_FIter std::remove_if(_FIter, _FIter, _Predicate) [with _FIter = __gnu_cxx::__normal_iterator<std::unique_ptr<odfaeg::graphic::LightComponent>*, std::vector<std::unique_ptr<odfaeg::graphic::LightComponent> > >; _Predicate = odfaeg::graphic::LightComponent::removeChild(odfaeg::graphic::LightComponent*)::<lambda(const std::unique_ptr<odfaeg::graphic::LightComponent*>&)>]’|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/lightComponent.h|119|required from here|
/usr/include/c++/4.9/bits/predefined_ops.h|231|error: no match for call to ‘(odfaeg::graphic::LightComponent::removeChild(odfaeg::graphic::LightComponent*)::<lambda(const std::unique_ptr<odfaeg::graphic::LightComponent*>&)>) (std::unique_ptr<odfaeg::graphic::LightComponent>&)’|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/lightComponent.h|117|note: candidate is:|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/lightComponent.h|117|note: odfaeg::graphic::LightComponent::removeChild(odfaeg::graphic::LightComponent*)::<lambda(const std::unique_ptr<odfaeg::graphic::LightComponent*>&)>|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/lightComponent.h|117|note:   no known conversion for argument 1 from ‘std::unique_ptr<odfaeg::graphic::LightComponent>’ to ‘const std::unique_ptr<odfaeg::graphic::LightComponent*>&’|
/usr/include/c++/4.9/bits/predefined_ops.h||In instantiation of ‘bool __gnu_cxx::__ops::_Iter_equals_val<_Value>::operator()(_Iterator) [with _Iterator = __gnu_cxx::__normal_iterator<odfaeg::graphic::LightComponent* const*, std::vector<odfaeg::graphic::LightComponent*> >; _Value = odfaeg::graphic::LightComponent** const]’:|
/usr/include/c++/4.9/bits/stl_algo.h|120|required from ‘_RandomAccessIterator std::__find_if(_RandomAccessIterator, _RandomAccessIterator, _Predicate, std::random_access_iterator_tag) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<odfaeg::graphic::LightComponent* const*, std::vector<odfaeg::graphic::LightComponent*> >; _Predicate = __gnu_cxx::__ops::_Iter_equals_val<odfaeg::graphic::LightComponent** const>]’|
/usr/include/c++/4.9/bits/stl_algo.h|162|required from ‘_Iterator std::__find_if(_Iterator, _Iterator, _Predicate) [with _Iterator = __gnu_cxx::__normal_iterator<odfaeg::graphic::LightComponent* const*, std::vector<odfaeg::graphic::LightComponent*> >; _Predicate = __gnu_cxx::__ops::_Iter_equals_val<odfaeg::graphic::LightComponent** const>]’|
/usr/include/c++/4.9/bits/stl_algo.h|3780|required from ‘_IIter std::find(_IIter, _IIter, const _Tp&) [with _IIter = __gnu_cxx::__normal_iterator<odfaeg::graphic::LightComponent* const*, std::vector<odfaeg::graphic::LightComponent*> >; _Tp = odfaeg::graphic::LightComponent**]’|
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/lightComponent.h|118|required from here|
/usr/include/c++/4.9/bits/predefined_ops.h|191|error: comparison between distinct pointer types ‘odfaeg::graphic::LightComponent*’ and ‘odfaeg::graphic::LightComponent**’ lacks a cast [-fpermissive]|
||=== Build failed: 2 error(s), 19 warning(s) (0 minute(s), 6 second(s)) ===|

Je ne sais pas comment il a fait le gaz mais chez moi ça ne fonctionne pas, je vais essayer ta solution.

Invité · 09/01/2019, 17h49

Non c'est bon je m'étais trompé j'avais déclaré un std::unique_ptr sur une pointeur.

Code :

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

1
2
std::unique_ptr<LightComponent*>
std::unique_ptr<LightComponent>

**r0d** · 09/01/2019, 18h02

Je ne comprends pas le but de la manoeuvre, mais soit.
Sinon tu as std::experimental::erase_if() qui peut simplifier grandement le code, et tu feras pas mieux en terme de perf.

Invité · 09/01/2019, 18h11

Oui le code précédent ce n'est pas le mieux en terme de perfs il le dit lui même.
Je vais essayé std::experimental::erase_if.

Invité · 09/01/2019, 18h17

Heu..., c'est une extension de la lib standart je ne la possède pas :

Code :

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

1
2
 
/home/laurent/Développement/Projets-c++/ODFAEG/src/odfaeg/Core/../../../include/odfaeg/Core/../Graphics/../../../include/odfaeg/Graphics/lightComponent.h|109|error: ‘std::experimental’ has not been declared|

Comment on fait pour la télécharger sur ubuntu ?

**Bousk** · 09/01/2019, 18h27

Envoyé par r0d

tout d'abord, tu ne peux pas supprimer un élément de ton vector dans la boucle for qui itère sur tout le vector.

Le code est correct, erase retourne l'itérateur suivant.

Pour le reste, kamoulox habituel de Laurent7601
- le code ne compile pas

- unique_ptr et pointeurs nus sur le même pointeur

- unique_ptr possède une fonction get qui pourrait permettre un simple ==

**r0d** · 09/01/2019, 18h36

Envoyé par Bousk

Le code est correct, erase retourne l'itérateur suivant.

Ha oui, tu as raison. Autant pour moi.
Par contre il re-alloue un nouveau vector après chaque erase, pour conserver l'invariant de contigüité des éléments, donc c'est bien atroce en terme de perfs.

**JolyLoic** · 09/01/2019, 18h37

Envoyé par Bousk

Le code est correct, erase retourne l'itérateur suivant.

Pour être précis, il est possible d'effacer les éléments d'un vecteur sur lequel on est en train de boucler, mais :
- C'est pas si trivial
- Vraiment pas (certains code qui ont l'air de marcher échouent si on doit effacer deux éléments consécutifs, ou si on doit effacer le dernier...)
- Même si on écrit le code correctement, c'est irresponsable, car ce sera en O(n²), là ou un erase-remove est en O(N).

**r0d** · 09/01/2019, 18h43

A-t-on les mêmes problèmes avec les autres conteneurs?
Y a-t-il de la littérature à ce sujet? Parce que par exemple, sur cppreference, ils ne parlent pas de ces problèmes-là. Et c'est un sujet assez récurrent.

**Pyramidev** · 09/01/2019, 20h05

Bonjour,

Laurent7601, d'après ce que je comprends, tu as une certaine classe qui contient une variable membre std::vector<std::unique_ptr<LightComponent>> children et tu veux lui ajouter une fonction membre removeChild qui retire et détruit un de ces enfants identifié à partir de son adresse.

Donc, algorithmiquement, quand tu parcours ton vecteur dans l'ordre, dès que tu as trouvé l'enfant, ce n'est pas la peine de parcourir le reste du vecteur. Aucun des std::unique_ptr<LightComponent> suivants ne peux lui aussi pointer sur le même objet. Sinon, ça voudrait dire qu'il y aurait une erreur de programmation en amont.

Du coup, le code est simplement :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
[[nodiscard]] bool removeChild(LightComponent& child)
{
	const auto itChildToRemove =
		std::find_if(children.begin(), children.end(),
		             [&](auto& p) { return p.get() == &child; });
 
	const bool found = (itChildToRemove != children.end());
	if(found)
		children.erase(itChildToRemove);
	return found;
}

Remarques :

Il faut aussi réfléchir au cas où l'enfant n'est pas trouvé. Du coup, j'ai retourné bool au lieu de void.
Mais, le problème des valeurs retournées, c'est qu'on peut les ignorer par mégarde. Pour réduire les risques d'erreur de programmation du code appelant, j'ai ajouté un attribut [[nodiscard]], disponible depuis C++17, qui demande au compilateur d'afficher un avertissement si l'appelant a ignoré implicitement la valeur retournée.
Si l'appelant veut vraiment ignorer la valeur retournée, il peut le faire, mais de manière explicite. En effet, en convertissant explicitement la valeur retournée en void, cela fera disparaître l'avertissement.
À la place d'un paramètre de type pointeur, j'ai mis un paramètre de type référence, car pouvoir passer nullptr à cette fonction n'a pas vraiment de sens.

Invité · 12/01/2019, 02h39

C'est ce que je cherchais à faire, merci. :B

Comment erase un std::unique_ptr d'un vecteur en fournissant son pointeur nu ?

Langage C++

Discussions similaires

Partager

Partager