станд :: Карта inizialitazion (только один раз)

Question

У меня есть функция, которая переводит данные, используя зЬй :: Карта

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

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

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

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

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

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, как предложено Niel.

Answer 3

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

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

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

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

затем:

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

Примечание Вам не нужно явное преобразование станд :: строка. Или еще лучше:

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

Answer 4

Я есть зЬй :: Карта < станд :: строка, 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 и спаривание ключ «тел» с этим последним объектом, оставляя предыдущий один болтается. При создании нового объекта каждый раз, когда ваше действительное намерение, вы, вероятно, нужно вызвать

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

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

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