2010-11-12 18 views
25

Mon instinct est que ce n'est pas le cas. Je suis dans la situation suivante:Est-il correct d'appeler une fonction dans la liste d'initialisation du constructeur?

class PluginLoader 
{ 
    public: 
     Builder* const p_Builder; 
     Logger* const p_Logger; 

     //Others 
}; 

PluginLoader::PluginLoader(Builder* const pBuilder) 
    :p_Builder(pBuilder), p_Logger(pBuilder->GetLogger()) 
{ 
    //Stuff 
} 

Ou devrais-je changer le constructeur et passer un Logger* const d'où PluginLoader est construit?

Répondre

27

C'est parfaitement bien et normal. p_Builder a été initialisé avant elle.

+0

Gee. Je dois être une porte "pas". :) – nakiya

+11

C'est encore plus vrai, puisqu'il appelle 'pBuilder-> GetLogger()', pas 'p_Builder-> GetLogger()'. Les deux sont légaux, mais le second est sensible à la variable d'instance réorganisée dans la définition de classe. – Eclipse

+0

@Eclipse: Oh, je n'ai même pas vu ça. @nakiya: Vouliez-vous utiliser le membre ou le paramètre? L'un ou l'autre est sûr, tel quel. – GManNickG

0

Parfaitement bonne pratique.

Je suggère cela (mais son à un niveau purement personnel):

au lieu d'avoir des fonctions appelées dans votre constructeur, de les regrouper dans une fonction init, uniquement à des fins de flexibilité: si vous avez plus tard pour créer autres constructeurs.

+3

Si ce que vous suggérez est une fonction * init * privée que vous appelez du constructeur, je ne m'en soucie pas vraiment mais je suppose que c'est ok. Si ce que vous suggérez est une fonction * public * 'init' qui doit être appelée par les utilisateurs de la classe, elle est sujette aux erreurs et aux incohérences (après avoir créé un objet, elle serait dans un état invalide et l'utilisateur peut oublier pour appeler le 'init' ou essayer d'utiliser l'objet entre la construction et l'initialisation ...) –

+0

vous pouvez utiliser un membre factice, et avoir la méthode init retourne un type.Par exemple 'class IDGenerator { public: IDGenerator(); réinitialisation booléenne(); std :: uint32_t generate(); privé: bool mReset; T_ID_set mIds; std :: uint32_t mID; }; IDGenerator :: IDGenerator() : mReset (reset()) { } bool IDGenerator :: reset() { mIds.clear(); mID = 0; renvoyer true; } ' – dgsomerton

19

Ce que vous avez est bon. Cependant, je veux juste vous avertir de faire attention ne pas faire cette: (GMan fait allusion à cela, je voulais juste qu'il soit parfaitement clair)

class PluginLoader 
{ 
    public: 
     Logger* const p_Logger; // p_Logger is listed first before p_Builder 
     Builder* const p_Builder; 

     //Others 
}; 

PluginLoader::PluginLoader(Builder* const pBuilder) 
    :p_Builder(pBuilder), 
    p_Logger(p_Builder->GetLogger()) // Though listed 2nd, it is called first. 
             // This wouldn't be a problem if pBuilder 
             // was used instead of p_Builder 
{ 
    //Stuff 
} 

Notez que j'ai fait 2 changements à votre code. Premièrement, dans la définition de classe, j'ai déclaré p_Logger avant p_Builder. Deuxièmement, j'ai utilisé le membre p_Builder pour initialiser p_Logger, au lieu du paramètre.

L'un de ces changements serait bien, mais ensemble, ils introduisent un bogue, car p_Logger est initialisé en premier, et vous utilisez le p_Builder non initialisé pour l'initialiser. Souvenez-vous toujours que les membres sont initialisés dans l'ordre dans lequel ils apparaissent dans la définition de la classe. Et l'ordre que vous les mettez dans votre liste d'initialisation est sans importance.