Bonsoir

Je me demande quelle est la meilleur façon d'écrire cette fonction membre ?

1 )

// add_fish
void app::aquarium::add_fish ( const std::string & name , const sex & s ) {
	fish_.push_back ( std::make_unique < fish > ( name , s ) );
}
//----------------------------------------------------------------------------------------------------------

2 )

// add_fish
void app::aquarium::add_fish ( const fish & f ) {
	fish_.push_back ( std::make_unique < fish > ( f.get_name () , f.get_sex () ) );
}
//----------------------------------------------------------------------------------------------------------

Merci

-
Edité par Severus Rogue 3 mai 2016 à 22:10:35

Par rapport à l'énnoncé, l'aquarium à une fonction pour ajouter un poisson, ce qui suppose que le poisson existe au préalable, donc la V1 est a éliminer (l'aquarium ne crée pas de poisson).
La V2 est a éliminer également, puisque l'aquarium crée un poisson à partir des données d'un poisson que tu lui donne en paramètre (ce qui revient à créer un clone).

De plus (gbdiver me l'avais notifié), utilises-tu l'heritage ? si non, il n'y a aucune raison d'utiliser des pointeurs.

-
Edité par Deedolith 3 mai 2016 à 23:09:00

D'accord, je vais essayer de faire les modifications en suivant t'es conseils

Merci

Re,

J'ai fait ça :

// add_fish
void app::aquarium::add_fish ( const fish & f ) {
	fish_.push_back ( std::move( f ) );
}
//------------------------------------------------------------------------------------------------

Je pense que c'est mieux, ont a le poisson existant en argument et ont le déplace dans le vecteur ( sans faire de clone ) enfin j'espère xD

Au début j'avais fait :

fish_.push_back ( f );

Mais la y a une copie nan ? ( constructeur de copie ? )

Merci

-
Edité par Severus Rogue 4 mai 2016 à 1:08:30

Std::move ou pas, il y aura une copie ici.

Pour rappel, le but de std::move est de modifier le type de référence d'une variable en une rvalue qui indique communément une variable temporaire. (Concrètement, std::move n'est rien de plus qu'un cast).

Comme la valeur est temporaire, on s'autorise une copie superficielle¹ et une remise à zéro des champs à risque histoire d'avoir un état cohérent (principalement pour que le destructeur fasse son job sans erreur).

¹copie superficielle: Par exemple, une copie d'un pointeur, mais, pas de l'élément pointé. Le pointeur a donc une double existence, d'où la remise à zéro dans la variable déplacée d'origine.

Sauf qu'une variable constante ne peut pas être remise à zéro ce qui rend impossible un déplacement, un T const && est finalement contradictoire. On peut le faire par contre... Il y a même des fonctions dans le standard avec une telle signature Oo.

Comme un constructeur avec T const && n'existe pas, mais que tous les types peuvent rentrer dans une référence constante, le compilateur se rabat sur le constructeur de copie et tout va bien pour le meilleur des mondes (celui fourbe du C++, j'entends).

Le truc drôle. c'est que prendre un type plein (cf. void add_fish(fish f)) peut ne pas faire de copie contrairement à l'idée reçut: cela dépend uniquement de la méthode de construction utilisée par l'appelant.

• app.add_fish({name, male}) pas de copie
• app.add_fish(std::move(my_fish)) pas de copie
• app.add_fish(my_fish) copie

Donc finalement, si tu ne veux pas de copie, il faut passer soit par référence et un conteneur de référence (cf. std::reference_wrapper), soit par pointeur et un conteneur de pointeur. Comme le l'aquarium devient propriétaire du poisson, le meilleur type pour représenter cette information est std::unique_ptr: void add_fish(std::unique_ptr<fish> && f) ou void add_fish(std::unique_ptr<fish> f). Personnellement, je préfère le premier prototype: dans le cas où une exception survient dans la fonction et que le pointeur n'est pas encore déplacé dans le vecteur, le poisson est toujours accessible depuis l’extérieur de la fonction (à condition que l'appelant l'ait mis quelque part avant de le déplacer).

-
Edité par jo_link_noir 4 mai 2016 à 3:08:36

Re ( oui oui je suis encore là ),

J'ai fait ça :

// add_fish 2
void app::aquarium::add_fish ( std::unique_ptr< fish > && f ) {
	fish_.push_back ( *f );
}
//------------------------------------------------------------------------------------------------

Et dans le main.cpp

	std::unique_ptr < app::fish > princesse { std::make_unique < app::fish > ( "Princesse", app::sex_fish::FEMALE ) };
	a.add_fish ( std::move ( princesse ) );

Donc la ça fait bien :

• Je fabrique le poisson dans le main
• Je le déplace dans ma fonction add_fish sans copie
• Et je le déplace ensuite dans le vecteur.

C'est correct ? y a t'il un moyen de vérifier dans le code que je ne produit aucune copie ?

Merci

C'est quoi le type de ton vector ?

Mon vector est :

std::vector < fish > fish_;

#include <string>
#include <deque>


struct Fish{
  Fish(const std::string & name_, bool sex_):name(name_),sex(sex_){}
  Fish(const Fish&)=default;  
  Fish(Fish&&)=default;

  std::string name;
  bool sex;
  static constexpr bool MALE   = true;
  static constexpr bool FEMALE = false;
};

struct Aquarium{

  void  addFish(const Fish &f){data.emplace_back(f);}
  Fish& newFish(const std::string &name, bool sex){data.emplace_back(name,sex); return data.back();}
  void  moveFish(Fish &&fg){data.emplace_back(std::move(f));}  

        Fish & getFish(size_t i)     {return(data[i]);}
  const Fish & getFish(size_t i)const{return(data[i]);}

  private:
  std::deque<Fish> data;
};


void test(){
  Aquarium a;

  Fish nemo("nemo",Fish::MALE);
  a.addFish(nemo);

  Fish & glouglou = a.newFish("glouglou",Fish::FEMALE);

  a.moveFish(Fish("blub",Fish::MALE));



}

-
Edité par ledemonboiteux 4 mai 2016 à 11:16:00

Severus Rogue a écrit:

Mon vector est :

std::vector < fish > fish_;

Du coup, ce n'est pas forcement cohérent de manipuler ton fish via pointeur en dehors de la fonction et sans pointeur dans ton vector. Sois il est nécessaire de manipuler ton fish par pointeur et tu le fais partout, soit ce n'est pas nécessaire et tu le fais nul part.

Si dans ma classe je fait :

std::vector < std::unique_ptr < fish > > fish_;

Dans ma fonction membre add_fish je dois faire un move aussi ?

// add_fish
void app::aquarium::add_fish ( std::unique_ptr< fish > && f ) {
	fish_.push_back ( std::move ( f ) );
}
//------------------------------------------------------------------------------------------------

 Et dans le main :

a.add_fish ( std::move ( std::unique_ptr < app::fish > { std::make_unique < app::fish > ( "Courgette" , app::sex_fish::FEMALE ) } ) );

Merci

-
Edité par Severus Rogue 4 mai 2016 à 11:39:31

Tant que tu n'as pas d'heritage, les pointeurs intelligents n'ont que peut de raison d'être employés.
Retire les, ca t'emmèle les pinceaux plus qu'autre chose.

@ledemonboiteux:
Pas très malin de poster une solution toute faite (qui plus est sans explications), niveau pédagogique, cela n'apporte rien.
L'être humain est fénéant par nature ===> copier / coller, ca marche, je suis content, mais j'ai pas appris ni compris.

J'avais carrément:

    template <typename LV, typename... Args>
        void add(Args&&... args) {
            newBorns<LV>().emplace_back(std::make_unique<LV>(std::forward<Args>(args)...));
            std::cout << "+ New: " << color::green << *newBorns<LV>().back() << "\n";
        }

    template <typename LV>
    typename std::enable_if<std::is_same<LV, Seaweed>::value, std::vector<std::unique_ptr<Seaweed>>&>::type
    newBorns()
    { return m_newSeaweeds; }

    template <typename LV>
    typename std::enable_if<std::is_same<LV, Fish>::value, std::vector<std::unique_ptr<Fish>>&>::type
    newBorns()
    { return m_newFishes; }

....


  aq.add<Fish>(0, "Tuna"       , Specy::Tuna, Sex::Male  );

J'avais poussé le vice à avoir une liste des nouveaux poissons, une liste des nouvelles algues, et encore une de chaque pour les pas nouvelles. Pas forcément le plus pratique, mais ça me simplifiait mes problèmes d'invalidation d'itérateurs, et cela me donnait un jour sans danger ni action pour les nouveaux nés.

Tout ça pour dire que push_back n'apporte rien par rapport à emplace_back ici. Et que ce que vous faites avec make_unique peut être étendu à n'importe qu'elle fonction générique.

-
Edité par lmghs 4 mai 2016 à 15:51:52

Salut,

Pour ce qui concerne la question d'origine, (faut il passer juste les variables qui permettent de créer un poisson, ou le poisson en lui-même), j'aurais tendance à dire que "cela dépend très fortement".

D'un coté, la loi de Déméter voudrait que tu ne force pas l'utilisateur de ta "liste de poissons" à connaitre "nommément" tous les poissons que tu pourrais vouloir ajouter à la liste.  Dés lors, elle t'orienterait volontiers vers une fonction proche de

void Aquarium::add(std::string const & name, Race const& race, Sexe sexe){
    fishes_.push_back({name, race, sexe});
}

D'un autre coté, on peut -- très clairement -- partir du principe que, si l'on ajoute un poisson à un aquarium, c'est qu'on l'a très certainement retiré d'un "autre environnement" dans lequel il existait déjà et que... cela n'a pas entrainé la mort du poisson (autrement, il aurait fini fissa à la poubelle).

Du coup, la seule véritable question que tu dois te poser c'est : est-ce que je me fous suffisamment de l'origine du poisson pour me permettre de ne le créer que quand j'en ai besoin ?

Si la réponse est oui, alors, autant ne transmettre que les informations de base permettant de créer le poisson.

Si la réponse est non (par exemple, parce que tu as deux aquarium et que tu veux pouvoir déplacer tes poissons de l'un à l'autre), alors, tu devras transmettre le poisson en lui-même, et la propriété qui va avec

Maintenant, si tu utilises les pointeurs (intelligents, de préférence) parce que tu prévois d'avoir plusieurs types de poissons présentant des comportements polymorphes (autrement, je ne vois pas pourquoi s'emmerder à utiliser des pointeurs ), je suis d'accord avec la remarque d'origine de Deetolith : la création du poisson en elle-même n'est pas du ressort de l'aquarium, mais bien de celui ... d'une fabrique de poisson.

Cependant, cela ne change pas vraiment le début de ma réponse (Déméter t'inciterait à ne transmettre que les "données de base" permettant la création du poisson), cela ne fait que justifier un changement en vue de... déléguer la création d'un poisson à la fabrique prévue à cet effet.  Si bien que le code que je viens de présenter serait sans doute modifier pour ressembler à

void Aquarium::add(std::string const & name, Race const& race, Sexe sexe){
    fishes_.emplace_back(std::move(Factory::create(name, race, sexe)));
}

(où Factory::create renverrait un unique_ptr sur le poisson fraichement créé)

Enfin, il est à noter que, même si tu ne te fous pas assez de l'origine du poisson que pour pouvoir l'ignorer, ils ne pourront de toutes façons pas apparaitre à leur "position d'origine" par "génération spontanée", si bien que tu devras quand même sans doute passer par la fabrique de poissons à un moment ou à un autre...

Je conclurais donc sous cette forme :

• quoi qu'il en soit, une fabrique de poissons semble être indispensable (SRP oblige)
• La "fabrique de poissons" ayant forcément besoin de "données de base" pour créer les poisson, il faudra pouvoir passer ces données à ta fonction add (qui déléguera à la fabrique la création proprement dite).
• Le cas échéant, tu peux également prévoir un "transfert de propriété" entre deux aquariums, pour que tu puisse "déménager" les poissons en les faisant passer d'un aquarium à un autre.  L'aquarium "de destination" devenant automatiquement le "propriétaire légitime" du poisson qu'il reçoit

Piouf, c'est difficile tout ça !

Je vais relire - relire et relire vos conseils pour essayer de pondre un nouveau code encore mieux

A dans quelques heures xD

-
Edité par Severus Rogue 4 mai 2016 à 16:36:13

edit

-
Edité par Severus Rogue 4 mai 2016 à 18:35:44

Mais pourquoi des pointeurs???

Sans polymorphisme, fais simple:

std::vector<Poisson> poissons;
Poisson p(nom, race, sexe);
poissons.push_back(std::move(p));

-
Edité par Deedolith 4 mai 2016 à 17:35:07

Deedolith a écrit:

Sans polymorphisme, fais simple:

std::vector<Poisson> poissons;
Poisson p(nom, race, sexe);
poissons.push_back(std::move(p));

-
Edité par Deedolith il y a environ 1 heure

C'est vrai que sa serais plus simple qu'avec tout les std::unique_ptr ...

Merci

Exactement, pas de pointeurs, pas de soucis.

Et pas de polymorphisme ... Poissons et algues sont comestibles.