std :: map inizialitazion (nur einmal)

Question

Ich habe eine Funktion, die Daten unter Verwendung von std :: map

übersetzt

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];
}

Meine Struktur ist sehr leicht, aber Bild kann es schwer sein. Die prolem ist, dass jedes Mal, wenn ich diese Funktion aufrufen es eine Menge erstellen von HistoParameter Objekten, vielleicht ein Schaltergehäuse ist effizienter. Erste Frage:? Ich erstelle Müll

Zweite Lösung:

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];

ist es ok? Bessere Lösung?

Answer 1

Ihre zweite Lösung sollte sicherlich die Effizienz zu verbessern, ist aber nicht (zumindest IMO), um die beste Umsetzung möglich. Zunächst einmal ist es first_time öffentlich sichtbar, auch nur wenn wirklich Sorgen darüber variable_to_parameter. Du hast bereits hp eine statische Variable in der Funktion gemacht und first_time sollte auch sein.

Zweitens würde ich nicht verwenden Zeiger und / oder dynamische Zuordnung für die HistoParameter Werte. An einem int und zwei Schwimmern, es gibt einfach keinen Grund, dies zu tun. Wenn Sie sie wirklich um vorbei so viel, dass das Kopieren ein Problem wurde, dann würden Sie wahrscheinlich besser dran, eine Art von Smart-Pointer-Klasse anstelle eines Rohzeiger mit - letzteres ist schwieriger zu verwenden und viel schwieriger zu machen Ausnahme sicher.

Drittens würde ich prüfen, ob es sich lohnt variable_to_parameter in eine Funktors statt einer Funktion zu machen. In diesem Fall würden Sie die Karte in dem Ctor initialisieren, so würden Sie nicht, ob es wurde jedes Mal operator() aufgerufen initialisiert haben zu prüfen, wurde. Sie können auch die beide kombinieren, um eine statische Karte in dem Funktors haben. Die Ctor initialisiert es, wenn es nicht vorhanden ist, und Operator () macht nur einen Nachschlag.

Schließlich würde ich fest, dass map::operator[] für Einfügen von Elementen in erster Linie nützlich ist - sie schafft ein Element mit dem angegebenen Schlüssel, wenn es nicht vorhanden ist, aber wenn Sie nach einem Elemente suchen, Sie in der Regel nicht wollen ein Element zu erstellen. Dafür sind Sie besser im Allgemeinen aus map.find() anstelle.

Answer 2

Die zweite Lösung OK mir scheint - man kann sagen:

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

Ich würde auch halten es für eine Karte von Werten statt Zeiger machen - ich sehe nicht, dass Sie die dynamische Zuordnung müssen hier:

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

dann:

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

Beachten Sie nicht die explizite std :: string brauchen Konvertierung. Oder noch besser:

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

Answer 3

Die erste Lösung erzeugt eine Menge Müll. Warum Sie nicht die Klasse als Wert zurückgeben? Es ist ganz leicht, und Sie wouln't dynamisch zuweisen müssen.

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];
}

Wenn die Klasse zurückgegeben wird größer, und Sie wollen ein Power-Tool, dann versuchen, boost :: Fliegengewicht .

Wenn Sie nicht über eine große Struktur zurücklaufen möchten, können Sie tun:

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

... und sogar in einem const werfen, wenn Sie es unveränderlich wollen.

Edit:. hinzugefügt make_pair, wie von Niel vorgeschlagen

Answer 4

Ich würde eine std :: map Mitglied und tun

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

Und dann

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

Answer 5

oder so, erstellen Sie Speicherleck. jedes Mal, wenn der = Operator, zum Beispiel genannt:

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

Sie erstellen ein neues Objekt HistoParameter und Paarung die Taste „ph“ mit diesem jüngsten Objekt, das vorherige baumelnden verlassen. Wenn ein neues Objekt erstellt jedes Mal ist Ihre eigentliche Absicht, Sie wahrscheinlich anrufen müssen

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

vor dem new Betrieb.

ist mein Vorschlag roh new Operationen so weit wie möglich zu vermeiden und greift auf intelligente Zeiger wie boost :: share_ptr für die Objektlebenszeitmanagement.