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

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

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

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

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

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

Answer 2

Второе решение мне кажется приемлемым — вы можете сказать:

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 3

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

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

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

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

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

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

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

Answer 4

У меня был бы член std::map< std::string, HistoParameter *> и я бы сделал

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

А потом

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

Answer 5

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

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

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

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

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

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