std :: map inizialitazion (только один раз)

Question

У меня есть функция, которая переводит данные, используя 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];
}

Моя структура очень легкая, но изображение может быть тяжелым. Дело в том, что каждый раз, когда я вызываю эту функцию, она создает много объектов HistoParameter, возможно, случай переключения более эффективен. Первый вопрос: я создаю мусор?

Второе решение:

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

это нормально? Лучшее решение?

Answer 1

Второе решение мне подходит - вы можете сказать:

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

Я бы также подумал о том, чтобы сделать это картой значений, а не указателей - я не вижу необходимости динамического размещения здесь:

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

тогда:

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

Обратите внимание, что вам не нужно явное преобразование std :: string. Или еще лучше:

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

Answer 2

Первое решение производит много мусора. Почему бы вам не вернуть класс по значению? Он довольно легкий, и вам не нужно его динамически распределять.

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

Если возвращаемый класс становится больше и вам нужен электроинструмент, попробуйте boost :: flyweight .

Если вы не хотите возвращать большую структуру, вы можете сделать:

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

... и даже добавьте const, если хотите, чтобы он был неизменным.

Редактировать: добавлена ​​make_pair, как предложил Ниль.

Answer 3

Ваше второе решение, безусловно, должно повысить эффективность, но не является (по крайней мере, IMO) наилучшим возможным вариантом реализации. Прежде всего, это делает first_time публично видимым, хотя на самом деле только variable_to_parameter заботится об этом. Вы уже сделали hp статической переменной в функции, и first_time также должно быть.

Во-вторых, я бы не использовал указатели и / или динамическое распределение для значений HistoParameter. В одном int и двух float просто нет причин делать это. Если вы действительно так много раздаете их, что копирование стало проблемой, вам, вероятно, было бы лучше использовать некоторый класс интеллектуальных указателей вместо необработанного указателя - последний сложнее использовать и сделать намного сложнее исключение безопасно.

В-третьих, я бы подумал, стоит ли превращать variable_to_parameter в функтор вместо функции. В этом случае вы бы инициализировали карту в ctor, поэтому вам не пришлось бы проверять, инициализировалась ли она каждый раз, когда вызывался operator(). Вы также можете объединить их, имея статическую карту в функторе. Ctor инициализирует его, если он не существует, а operator () просто выполняет поиск.

Наконец, я бы отметил, что map::operator[] в первую очередь полезен для вставки элементов - он создает элемент с указанным ключом, если он не существует, но когда вы ищете элемент, вы обычно не Я не хочу создавать предмет. Для этого лучше использовать map.find().

Answer 4

в любом случае, вы создаете утечку памяти. каждый раз, когда вызывается оператор =, например:

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

вы создаете новый объект HistoParameter и соединяете ключ "ph" с этим самым последним объектом, оставляя предыдущий висячий. Если создание нового объекта каждый раз является вашим реальным намерением, вам, вероятно, нужно вызвать

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

до операции new.

Мое предложение - максимально избегать необработанных операций new и прибегать к умным указателям, таким как boost :: share_ptr , для управления временем жизни объекта.

Answer 5

Я бы имел элемент std :: map и сделал бы

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

А потом

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