1

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

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

4

5 に答える 5

4

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 )));
于 2010-01-25T22:18:27.477 に答える
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];
}

返されたクラスが大きくなり、動力工具が必要な場合は、 boost::flyweightを試してください。

大きな構造を返したくない場合は、次のようにすることができます。

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

const...不変にしたい場合は、をスローします。

編集：ニールによって提案されたように、make_pairを追加しました。

于 2010-01-25T22:18:26.367 に答える
1

2番目のソリューションは確かに効率を改善するはずですが、（少なくともIMOは）可能な限り最良の実装ではありません。まず第一に、それは実際にそれを気にするfirst_timeだけであるにもかかわらず、公に見えるようにします。関数で静的変数variable_to_parameterをすでに作成しているので、同様に作成する必要があります。hpfirst_time

次に、HistoParameter値にポインターや動的割り当てを使用しません。1つのintと2つのfloatでは、そうする理由はまったくありません。コピーが問題になるほど実際にそれらを渡している場合は、生のポインターの代わりに、ある種のスマートポインタークラスを使用する方がよいでしょう。後者は使用がより難しく、作成がはるかに困難です。例外安全。

第三に、variable_to_parameterを関数ではなくファンクターにする価値があるかどうかを検討します。この場合、ctorでマップを初期化するので、operator()呼び出されるたびにマップが初期化されたかどうかを確認する必要はありません。ファンクターに静的マップを設定することで、2つを組み合わせることもできます。ctorが存在しない場合は初期化し、operator（）はルックアップを実行します。

最後に、これmap::operator[]は主にアイテムの挿入に役立ちます。キーが存在しない場合は指定されたキーでアイテムを作成しますが、アイテムを探している場合は通常、アイテムを作成したくありません。 。このため、通常はmap.find()代わりに使用することをお勧めします。

于 2010-01-25T22:27:14.693 に答える
0

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];
}
于 2010-01-25T22:19:58.143 に答える
0

いずれにせよ、メモリリークが発生しています。演算子が呼び出されるたび=に、たとえば次のようになります。

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

新しいHistoParameterオブジェクトを作成し、キー「ph」をこの最新のオブジェクトとペアにして、前のオブジェクトをぶら下げたままにします。毎回新しいオブジェクトを作成することが実際の目的である場合は、おそらく呼び出す必要があります

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

手術前new。

私の提案は、生の操作をできるだけ避け、オブジェクトのライフタイム管理のためにboost::share_ptrnewなどのスマートポインターに頼ることです。

于 2010-01-26T04:31:42.150 に答える