std :: mapa inizialitazion (una sola vez)

Question

Tengo una función que traduce los datos usando std :: mapa

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

Mi estructura es muy ligero, pero la imagen que puede ser pesado. El prolem es que cada vez que llamo esta función se crea una gran cantidad de objetos HistoParameter, tal vez una caja de conmutación es más eficiente. Primera pregunta:? Estoy creando basura

Segunda solución:

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á bien? solución mejor?

Answer 1

Su segunda solución, sin duda debe mejorar la eficiencia, pero no es (al menos OMI) la mejor aplicación posible. En primer lugar, se hace first_time visible para el público, a pesar de que sólo se variable_to_parameter realmente se preocupa por ella. Que ya ha hecho hp una variable estática en la función, y first_time debe ser así.

En segundo lugar, yo no usaría punteros y / o la asignación dinámica de los valores HistoParameter. En un int y dos flotadores, simplemente no hay razón para hacerlo. Si usted está realmente pasándolas alrededor tanto que la copia se convirtió en un problema, probablemente sería mejor usar algún tipo de clase de puntero inteligente en lugar de un puntero cruda - este último es más difícil de usar y mucho más difícil hacer excepción de seguridad.

En tercer lugar, me gustaría considerar si vale la pena hacer variable_to_parameter en un funtor en lugar de una función. En este caso, usted inicializar el mapa en el ctor, por lo que no tendría que comprobar si éste fue inicializado cada vez que se invocó operator(). También se pueden combinar los dos, por tener un mapa estático en el funtor. La Héctor inicializa si no existe, y el operador () simplemente realiza una búsqueda.

Por último, me gustaría tener en cuenta que map::operator[] es principalmente útil para insertar elementos - que crea un elemento con la clave especificada si no existe, pero cuando estás en busca de un elemento, por lo general, no quiero para crear un elemento. Para esto, usted es generalmente mejor usar map.find() lugar.

Answer 2

La segunda solución parece bien para mí - se puede decir:

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

Yo también consideraría por lo que es un mapa de valores en lugar de punteros - No veo que necesita la asignación dinámica aquí:

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

a continuación:

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

Tenga en cuenta que no es necesario la conversión std :: string explícita. O mejor aún:

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

Answer 3

La primera solución produce una gran cantidad de basura. ¿Por qué no regresa a la clase de valor? Es bastante ligero, y que wouln't que asignar dinámicamente a él.

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 clase regresado hace más grande, y se desea una herramienta de poder, entonces probar impulso :: peso mosca .

Si no desea pasar de nuevo una estructura grande, que puede hacer:

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

... e incluso lanzar en un const si lo desea inmutable.

Editar:. make_pair añadido, según lo sugerido por Niel

Answer 4

Me tendría un std :: mapa miembros y hacer

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

y después

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

Answer 5

De cualquier manera, usted está creando pérdida de memoria. cada vez que el operador = se llama, por ejemplo:

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

se crea un nuevo objeto HistoParameter y el emparejamiento de la tecla "ph" con este objeto más reciente, dejando la anterior colgando. Si la creación de un nuevo objeto cada vez que es su verdadera intención, es probable que tenga a la llamada

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

antes de la operación new.

Mi sugerencia es evitar operaciones new primas tanto como sea posible y recurrir a los punteros inteligentes, como impulso :: share_ptr para la gestión del tiempo de vida del objeto.