std :: carte inizialitazion (une seule fois)

Question

J'ai une fonction qui convertit les données en utilisant std :: map

struct HistoParameter
{
  int nbins;
  float first;
  float last;
  HistoParameter(int _nbins, int _first, int _last) :
    nbins(_nbins), first(_first), last(_last) {};
};

HistoParameter* variable_to_parameter(char* var_name)
{
  std::map<const std::string, HistoParameter*> hp;
  hp[std::string("ph_pt")] = new HistoParameter(100,0,22000);
  hp[std::string("ph_eta")] = new HistoParameter(100,-3,3);
  // ...
  return hp[var_name];
}

Mon struct est très léger, mais l'image peut être lourd. Le prolem est que chaque fois que j'appelle cette fonction, il crée beaucoup d'objets HistoParameter, peut-être un cas de commutation est plus efficace. Première question: Je crée des déchets

Deuxième solution:

bool first_time = true;
HistoParameter* variable_to_parameter(char* var_name)
{
  static std::map<const std::string, HistoParameter*> hp;
  if (first_time)
    {
  hp[std::string("ph_pt")] = new HistoParameter(100,0,22000);
  hp[std::string("ph_eta")] = new HistoParameter(100,-3,3);
  // ...
    }
  first_time = false;
  return hp[var_name];

est-il correct? Une meilleure solution?

Answer 1

Votre deuxième solution devrait certainement améliorer l'efficacité, mais pas (au moins OMI) la meilleure application possible. tout, il fait d'abord first_time publiquement visible, même si seulement variable_to_parameter se soucie vraiment à ce sujet. Vous avez déjà fait hp une variable statique dans la fonction, et first_time devrait être aussi bien.

Deuxièmement, je ne voudrais pas utiliser des pointeurs et / ou l'allocation dynamique pour les valeurs de HistoParameter. A un int et deux flotteurs, il n'y a tout simplement aucune raison de le faire. Si vous êtes vraiment les passer autour tellement que la copie est devenu un problème, vous seriez probablement mieux d'utiliser une sorte de classe de pointeur intelligent au lieu d'un pointeur brut - celui-ci est plus difficile à utiliser et beaucoup plus difficile à faire exception en toute sécurité.

Troisièmement, je pense que cela vaut la peine de faire variable_to_parameter dans un foncteur au lieu d'une fonction. Dans ce cas, vous initialiser la carte dans le cteur, de sorte que vous n'auriez pas à vérifier si elle a été initialisé à chaque fois operator() a été invoqué. Vous pouvez également combiner les deux, par une carte statique dans le foncteur. Le cteur initialise si elle n'existe pas, et l'opérateur () fait juste une recherche.

Enfin, je note que map::operator[] est principalement utile pour insérer des éléments - il crée un élément avec la clé spécifiée si elle n'existe pas, mais quand vous êtes à la recherche d'un élément, vous ne voulez pas habituellement pour créer un élément. Pour cela, vous êtes généralement mieux d'utiliser à la place map.find().

Answer 2

La deuxième solution me semble OK - vous pouvez dire:

if ( hp.empty() ) {
   // populate map
}

Je voudrais aussi envisager d'en faire une carte des valeurs plutôt que des pointeurs - Je ne vous vois pas besoin allocation dynamique ici:

 std::map <std::string, HistoParameter> hp;

alors:

 hp["ph_pt"] = HistoParameter(100,0,22000);

Notez que vous n'avez pas besoin explicite std :: conversion de chaîne. Ou mieux encore:

 hp.insert( std::make_pair( "ph_pt", HistoParameter(100,0,22000 )));

Answer 3

La première solution produit beaucoup de déchets. Pourquoi ne pas retourner la classe par valeur? Il est tout à fait léger, et vous devez wouln't allouer dynamiquement.

HistoParameter variable_to_parameter(char* var_name)
{
  static std::map<const std::string, HistoParameter> hp;
  if ( hp.empty() )
  {
    hp.insert( std::make_pair( "ph_pt", HistoParameter(100,0,22000) ) );
    hp.insert( std::make_pair( "ph_eta", HistoParameter(100,-3,3) ) );
  //...
  }
  return hp[var_name];
}

Si la classe retournée devient plus grande, et que vous voulez un outil, essayez ensuite boost :: poids mouche .

Si vous ne voulez pas passer de nouveau une grande structure, vous pouvez faire:

HistoParameter& variable_to_parameter(char* var_name)
{
  // same code
}

... et même jeter dans un const si vous le voulez immuable.

Modifier ajouté make_pair, comme suggéré par Niel

.

Answer 4

J'ai std :: map membre et faire

InitializeHistoParameter() 
{
   myMap["ph_pt"] = new ...
   myMap["ph_eta"] = new ...
}

Et puis

HistoParameter* variable_to_parameter(char* var_name) 
{
    return myMap[var_name];
}

Answer 5

De toute façon, vous créez fuite de mémoire. chaque fois que l'opérateur = est appelé, par exemple:

hp[std::string("ph_pt")] = new HistoParameter(100,0,22000);

vous créez un nouvel objet HistoParameter et jumelant la touche « ph » avec cet objet le plus récent, en laissant la précédente ballants. Si vous créez un nouvel objet à chaque fois est votre intention réelle, vous avez probablement besoin d'appeler

delete hp[std::string("ph_pt")]; 

avant l'opération de new.

Ma suggestion est d'éviter les opérations de new premières autant que possible et de recourir à des pointeurs intelligents tels que boost :: share_ptr pour la gestion du temps de vie de l'objet.