std::map の初期化 (1 回のみ)

StackOverflow https://stackoverflow.com/questions/2136053

  •  22-09-2019
  •  | 
  •  

質問

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 オブジェクトが作成されることです。おそらく switch ケースの方が効率的です。最初の質問:私がゴミを作っているのですか？

2 番目の解決策:

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

大丈夫ですか？より良い解決策は？

役に立ちましたか？

解決

あなたの第2の解決策は確かに効率を向上させる必要がありますが、可能な限り最高の実装（少なくともIMO）ではありません。まず第一に、それは、たとえだけ実際にそれを気をfirst_time、variable_to_parameterが公に表示されるようになります。あなたは既に機能でhp静的変数を作った、とfirst_timeは同様である必要があります。

第二に、私はHistoParameter値のポインタ、および/または動的な割り当てを使用することはありません。 1つのint型と2つのfloatで、そうする理由は、単純にありません。あなたが本当にそんなにコピーが問題となったことを周りにそれらを渡している場合は、おそらく代わりに生のポインタのスマートポインタクラスのいくつかの並べ替えを使用したほうが良いだろう - 後者は使用することがより困難とはるかに困難にすることです例外安全ます。

第三には、私はそれではなく、機能のファンクタにvariable_to_parameterを作るために価値があるかどうかを検討したいです。あなたはそれがoperator()が呼び出されたたびに初期化されたかどうかをチェックする必要はないので、この場合、あなたは、ctorの中でマップを初期化すると思います。ファンクタの静的マップを持っていることにより、また、両者を組み合わせることができます。 ctorのは、それが存在せず、オペレータが（）だけで検索を実行した場合、それを初期化します。

そのmap::operator[]項目を挿入するための、主に有用である。最後に、私は注意したい - それが存在しない場合には、指定されたキーを持つ項目が作成されますが、あなたがアイテムを探しているとき、あなたは通常はしたくありませんアイテムを作成することができます。このためには、一般的に、より良い代わりにmap.find()を使用してオフにしている。

他のヒント

第2の解決策は、私にはOKらしい - あなたが言うことができます：

if ( hp.empty() ) {
   // populate map
}
私もポインタではなく、それを値のマップを作る検討する - 私はあなたがここに動的な割り当てを必要と表示されていない。

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

、その後ます：

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

あなたは明示的にはstd ::文字列変換は必要ありません。またはより良いまだ：

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

最初の解決策では、大量のゴミが生成されます。クラスを値で返さないのはなぜですか?これは非常に軽量であり、動的に割り当てる必要はありません。

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 不変にしたい場合。

編集： Niel の提案に従って、make_pair を追加しました。

私はSTDを持っていると思います::マップ<はstd ::文字列、HistoParameter *>メンバーおよびDO 

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

そして

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

のいずれかの方法で、あなたはメモリリークを作成しています。 =オペレータが呼び出されるたびに、例えば

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

あなたは前のダングリングを残し、新しいHistoParameterオブジェクトを作成し、この最新のオブジェクトを持つキー「pHは」ペアリングされています。 毎回新しいオブジェクトを作成すると、あなたの実際の意図である場合は、おそらく

呼び出す必要があります 
delete hp[std::string("ph_pt")];

new操作の前に。

私の提案は、可能な限り生new操作を避けると、このような<のhref = "http://www.boost.org/doc/libs/1_41_0/libs/smart_ptr/shared_ptr.htmとしてスマートポインタに頼ることです"REL =" nofollowをnoreferrer ">ブースト:: share_ptr のオブジェクトのライフタイム管理のため。

ライセンス： CC-BY-SA と 帰属
所属していません StackOverflow
scroll top