Pergunta

Eu tenho uma função que traduz dados usando 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];
}

Minha estrutura é muito leve, mas a imagem pode ser pesada.O problema é que toda vez que eu chamo essa função ela cria muitos objetos HistoParameter, talvez um switch case seja mais eficiente.Primeira pergunta:Estou criando lixo?

Segunda solução:

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á tudo bem?Melhor solução?

Foi útil?

Solução

Sua segunda solução certamente deve melhorar a eficiência, mas não é (pelo menos IMO) a melhor implementação possível. Primeiro de tudo, faz first_time publicamente visível, embora apenas variable_to_parameter Na verdade, se preocupa com isso. Você já fez hp uma variável estática na função e first_time deve ser também.

Segundo, eu não usaria ponteiros e/ou alocação dinâmica para os valores de histoparâmetro. Em uma int e dois carros alegóricos, simplesmente não há razão para fazê -lo. Se você está realmente passando tanto que a cópia se tornou um problema, provavelmente seria melhor usar algum tipo de classe de ponteiro inteligente em vez de um ponteiro cru - o último é mais difícil de usar e muito mais difícil de fazer exceção segura.

Terceiro, eu consideraria se vale a pena transformar variable_to_parameter em um functor em vez de uma função. Nesse caso, você inicializaria o mapa no CTOR, para não precisar verificar se ele era inicializado toda vez operator() foi invocado. Você também pode combinar os dois, com um mapa estático no functor. O CTOR o inicializa se não existir, e o Operator () apenas faz uma pesquisa.

Finalmente, eu notaria que map::operator[] é principalmente útil para inserir itens - ele cria um item com a tecla especificada, se não existir, mas quando você está procurando um item, geralmente não deseja criar um item. Para isso, você geralmente é melhor usando map.find() em vez de.

Outras dicas

Vá para o seu navegador e clique na seta para modificar a Web Part.Expanda a seção avançada e substitua o URL do título com o URL desejado.

A primeira solução produz muito lixo.Por que você não retorna a classe por valor?É bastante leve e você não precisa alocá-lo 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 a classe retornada ficar maior e você quiser uma ferramenta elétrica, experimente boost::peso mosca.

Se não quiser devolver uma estrutura grande, você pode fazer:

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

...e até mesmo jogar um const se você quiser que seja imutável.

Editar: adicionado make_pair, conforme sugerido por Niel.

Eu teria um std :: map <std :: string, histoparameter *> membro e fazer

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

E depois

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

De qualquer forma, você está criando vazamento de memória. cada vez que o = O operador é chamado, por exemplo:

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

Você está criando um novo objeto de histoparâmetro e emparelhando a chave "pH" com esse objeto mais recente, deixando o anterior. Se criar um novo objeto cada vez é sua intenção real, você provavelmente precisa ligar

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

antes de o new Operação.

Minha sugestão é evitar cru new operações o máximo possível e recorrem a ponteiros inteligentes, como Boost :: share_ptr para gerenciamento de tempo de vida objeto.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top