std :: map inizialitazion (solo una volta)

Question

Ho una funzione che converte i dati utilizzando 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];
}

Il mio struct è molto leggero, ma l'immagine può essere pesante. Il prolem è che ogni volta che io chiamo questa funzione creare un sacco di oggetti HistoParameter, forse un caso interruttore è più efficiente. Prima domanda:? Sto creando spazzatura

Seconda soluzione:

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

è ok? Meglio soluzione?

Answer 1

La seconda soluzione dovrebbe certamente migliorare l'efficienza, ma non è (almeno IMO) possibile la migliore realizzazione. Prima di tutto, rende first_time pubblicamente visibili, anche se solo variable_to_parameter si preoccupa in realtà su di esso. Hai già fatto hp una variabile statica nella funzione, e first_time dovrebbe essere così.

In secondo luogo, non vorrei usare puntatori e / o l'allocazione dinamica per i valori HistoParameter. A un int e due galleggianti, semplicemente non c'è alcun motivo per farlo. Se sei veramente passarli in giro così tanto che la copia è diventato un problema, si sarebbe probabilmente meglio utilizzare una sorta di intelligente classe puntatore invece di un puntatore prime - il secondo è più difficile da usare e molto più difficile da fare un'eccezione di sicurezza.

In terzo luogo, mi piacerebbe prendere in considerazione se vale la pena di fare variable_to_parameter in un funtore invece di una funzione. In questo caso, devi inizializzare la mappa nel ctor, quindi non si dovrà verificare se è stato inizializzato ogni volta operator() è stato invocato. È inoltre possibile combinare le due cose, per avere una mappa statica nel funtore. Il ctor inizializza se non esiste, e l'operatore () solo fa una ricerca.

Infine, mi piacerebbe notare che map::operator[] è utile soprattutto per l'inserimento di oggetti - si crea un elemento con la chiave specificata, se non esiste, ma quando siete alla ricerca di un elemento, di solito non voglio per creare un oggetto. Per questo, si è generalmente meglio utilizzare map.find() invece.

Answer 2

La seconda soluzione sembra OK per me - si può dire:

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

Vorrei anche considerare la possibilità di una mappa di valori piuttosto che puntatori - non vedo è necessario l'allocazione dinamica qui:

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

quindi:

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

Si noti che non è necessario la conversione esplicita std :: string. O meglio ancora:

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

Answer 3

La prima soluzione produce un sacco di immondizia. Perché non ritorni la classe per valore? E 'molto leggero, e si wouln't deve allocare dinamicamente.

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

Se la classe restituita diventa più grande, e si desidera un potere-tool, quindi provare boost :: peso mosca .

Se non si desidera passare di nuovo una grande struttura, si può fare:

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

... e anche gettare in una const se lo vuoi immutabile.

Modifica make_pair aggiunto, come suggerito da Niel

.

Answer 4

avrei uno std :: map membro e fare

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

E poi

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

Answer 5

in entrambi i casi, si sta creando perdita di memoria. ogni volta che l'operatore è chiamato =, ad esempio:

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

si sta creando un nuovo oggetto HistoParameter e l'associazione il "ph" chiave con questo più recente oggetto, lasciando la precedente penzoloni. Se la creazione di un nuovo oggetto ogni volta è vostro intento reale, probabilmente è necessario chiamare

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

prima dell'operazione new.

Il mio suggerimento è quello di evitare le operazioni prime new il più possibile e ricorrere a puntatori intelligenti come ad esempio boost :: share_ptr per la gestione del tempo di vita dell'oggetto.