Discussion :

[julieng31]

Bonjour,

pour l'un des mes codes, je souhaite stocker des objets. Ils sont construits à la création de l'application et sont lus ensuite/modifiés par la suite (mais sans jamais toucher à la structure des objets). Je souhaite conserver l'ordre de création des objets, pouvoir itérer rapidement dessus et si possible pouvoir accéder aléatoirement rapidement à certains éléments. J'en arrive au code suivant :

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
 
class container {
public:
    container() {
                // Charger un ensemble de points (la classe point contient un attribut name dont la valeur est unique)
                for(__) {
                    m_points.push_back(std::unique_ptr<point>(new point(name));
                    m_pointsMap.emplace(name, std::ref(*m_points.back().get()));
                }
    }
 
    const std::vector<std::unique_ptr<point>> & getPoints() const {return m_points};
    const point & getPoint(const std::string & name) const {return m_pointsMap.at(name).get()}
    point & getPoint(const std::string & name) {return m_pointsMap.at(name).get()}
private:
    std::vector<std::unique_ptr<point>> m_points;
    std::map<std::string, std::reference_wrapper<point>> m_pointsMap;
}

J'aimerais savoir si cette approche est "bien écrite" et sinon, comment rendre ce code le plus propre possible (j'ai volontairement supprimé la gestion d'erreur dans le constructeur et les fonctions getPoint si le nom n'existe pas pour rendre le code simple).

D'autre part, ce qui me gène un peu est d'exposer ma structure de stockage au travers de getPoints (ie const std::vector<std::unique_ptr<point>>). Ce qui entraîne aussi des choses comme :

Code :

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

1
2
 
for(std::vector<std::unique_ptr<point>>::size_type i=0; i<getPoints().size(); i++)

où là aussi, je fais apparaître ma structure de stockage.

Si vous avez des conseils, je suis preneur ! Merci d'avance !!

[Bousk]

Oui vector est sûrement le mieux. vector doit de toutes façons toujours être le choix par défaut.
Pourquoi aurais-tu besoin d'exposer le vector de points ?
...::size_type est un simple size_t. Sinon il y a auto.
Et tu aurais tout intérêt à rendre ta classe container utilisable avec les ranged-for-loop.

[julieng31]

Merci Bousk.

Il me semblait que "...::size_type" était l'écriture la plus portable. Mais même si j'enlève ce point là, ma fonction :

Code :

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

1
2
 
const std::vector<std::unique_ptr<point>> & getPoints() const {return m_points};

expose tout de même ma structure de stockage à cause du type du retour de cette fonction (j'ai ajouté cette fonction pour les ranged-for loops).

[koala01]

Salut,

La bonne question est : pourquoi veux tu créer tes points en utilisant l'allocation dynamique de la mémoire

A priori, on n'utilise cette technique que pour les classes qui ont typiquement sémantique d'entité, afin de pouvoir profiter du polymorphisme. Or, une classe Point a -- typiquement -- sémantique de valeur, et il n'y a donc -- a priori -- aucune raison de vouloir recourir à l'allocation dynamique de la mémoire

un code proche de

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
class container {
public:
    container() {
                // Charger un ensemble de points (la classe point contient un attribut name dont la valeur est unique)
                for(__) {
                    m_points.push_back(point(name));
                    m_pointsMap.emplace(name, std::ref(*m_points.back()));
                }
    }
   /* pour profiter des itérateurs (constant ou non), au lieu de renvoyer le tableau entier */
    using iterator = typename std::vector<point>::iterator;
    using const_iterator = typename std::vector<point>::const_iterator;
    /* pour assurer l'accès en lecture + écriture */
    iterator begin(){
        return m_points.begin();
    }
    iterator end(){
        return m_points.end();
    }
    /* pour assurer l'accès en lecture seule */
    const_iterator begin() const{
        return m_points.begin();
    }
    const_iterator end() const{
        return m_points.end();
    }
    /* parce que l'on risque de vouloir poser certaines questions au container  comme*/
    // - savoir s'il est vide ou non 
    bool empty() const{
        return m_points.empty();
    }
    // - connaitre le nombre de points qu'il contient
    size_t size() const{
        return m_points.size();
    }
private:
    std::vector<point> m_points;
    std::map<std::string, std::reference_wrapper<point>> m_pointsMap;
};

Cela te simplifiera énormément la tâche par la suite

NOTA: au lieu de stocker un reference_wrapper dans m_pointsMap, vu que les références risquent fort d'être invalidées à chaque fois que la capacité de m_points devra augmenter pour ajouter un point, tu pourrait "simplement" stocker l'indice du point en question, qui sera, lui particulièrement constant

A moins, bien sur, que tu garantisse que ta classe container ne rajoute et ne supprime aucun point après l'étape de chargement. Dans ce cas, tu pourrait envisager d'utiliser un reference_wrapper, mais tu devrais travailler en deux temps :

1. dans un premier temps, tu remplis m_points sans t'inquiéter du contenu de m_pointsMap;
2. dans un deuxième temps, tu parcours l'ensemble de m_points pour remplir m_pointMap;

[julieng31]

Bonjour Koala01 et merci beaucoup pour cette réponse très claire. En fait, j'étais parti au départ sur une approche vecteur d'objets et non de pointeurs mais j'obtenais un segfault. Ta réponse me fait comprendre la raison maintenant (je mettais à jour m_points et m_pointsMap en même temps donc des références invalides) ! Après correction, ça marche bien (sachant que je ne mets pas à jour ces informations au cours de l’exécution du programme).

Maintenant, dans mon programme, j'ai aussi le cas avec où je pense devoir utiliser un vecteur de pointeurs (pour du polymorphisme). Dans ce cas, est-ce qu'il y aurait une "bonne approche" ?

Merci d'avance pour vos conseils !

[koala01]

Maintenant, dans mon programme, j'ai aussi le cas avec où je pense devoir utiliser un vecteur de pointeurs (pour du polymorphisme). Dans ce cas, est-ce qu'il y aurait une "bonne approche" ?

C'est, à vrai dire, toujours les mêmes conseils :

1. ainsi que l'as si bien dit Bousk : la collection par défaut sera un std::vector<XXX>
2. Toujours comme l'a fait remarquer Bousk (et comme je l'ai confirmé), on préférera fournir les fonctions begin et end, dans leur version constante et non constante
3. On utilisera toujours des pointeur intelligents
4. la classe qui s'occupe du maintien en mémoire des objets devrait ne faire que cela (en plus de fournir l'accès à ces données)
5. Autant que faire se peut, on préférera transmettre les objets sous forme de référence (éventuellement constante) que sous forme de pointeur
6. de manière tout à fait générale : on réfléchira à nos différentes classes en tant que fournisseurs de service au lieu de réfléchir en terme des données qui les composent
7. de manière tout à fait générale, on attachera une énorme importance au respect des principes SOLID (essentiellement S, O et L) et de la loi de Déméter

Pour le reste, énormément de choses vont dépendre de ton domaine d'application, de tes besoins et de ta situation actuelle

[julieng31]

Koala01, j'ai une dernière question. Si ma structure de stockage est un vecteur d'unique pointer (vos points 1 et 3 du dernier message) et que je mets en place les fonctions begin et end (point 2), il me semble que je ne respecte pas le point 5 (j'offre un itérateur sur des uniques pointers) -> est-ce que cette approche reste une bonne pratique ou qu'il existe une manière (que je ne connais pas) par begin et end de retourner des références ?

[koala01]

Koala01, j'ai une dernière question. Si ma structure de stockage est un vecteur d'unique pointer (vos points 1 et 3 du dernier message) et que je mets en place les fonctions begin et end (point 2), il me semble que je ne respecte pas le point 5 (j'offre un itérateur sur des uniques pointers)

Et tu as tout à fait raison C'est bien pour cela que tu devrai veiller aussi à respecter les points 6 (penser ta classe en termes de services)

est-ce que cette approche reste une bonne pratique ou qu'il existe une manière (que je ne connais pas) par begin et end de retourner des références ?

De fait, renvoyer une référence (un itérateur nétant qu'une sorte de référence bien particulière) sur un std::unique_ptr n'est jamais vraiment une bonne idée, à moins que la référence ne soit constante (ou que l'on prévoie de céder la propriété de l'objet pointé à la fonction qui obtient le unique_ptr ).

Mais tu as deux solutions, qui ne sont d'ailleurs pas anti-nomiques:

1. fournir une fonction size() et un opérateur [], de manière à permettre l'utilisation de boucles "classiques" ou
2. respecter le point 7 et principalement le DIP (même si j'ai par mégarde laisser sous-entendre qu'il était moins important dans mon intervention précédante).

Cela pourrait se faire sous une forme proche de

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
 /* préconditions : index < size()
 *                 tab[index] != nullptr
 */
Object /* const*/ & Container::operator[](size_t index) /* const */{
    assert(index < size() && "Index out of bound");
    assert(tab[index]!= nullptr && "invalid pointer");
    return *(tab[index].get());
}
/* pour permettre à n'importe quel foncteur de travailler (pour autant 
 * qu'il puisse travailler sur tous les types dérivés de Object
 */
template<typename Functor>
void Container::executeAll(Functor /* const */ & fun) /* const*/{
    for(auto /* const */ & it: tab){
        fun(*(it.get())):
    } 
}
 
/* OU OU OU, 
 * si la classe Object représente l'élément visitable du DP visiteur
 * et que l'on fournit une instance concrète de visiteur par référence
 */
void Container::executeAll(Visitor /* const*/ & v)/* const*/{
    for(auto /* const */ & it : tab)
        it.get()->accept(v);
}

Bien sur, il y a de sérieuses remarques à faire sur le DP visiteurs tel que présenté par le GoF à l'époque, mais le double dispatch, quelle que soit la forme qu'il puisse prendre, s'avère souvent être un allié des plus utiles quand on recours au polymorphisme

[julieng31]

Merci pour ces conseils qui m'ont bien éclairé !