2

最近、外部の会社から少し奇妙に思えるコードを入手しました。私の質問は次のとおりです。次のコード設計の良い点と悪い点を誰か説明してもらえますか?

があります

class A
{
  double val[2]; // actually within the original class there are: 
  // an int, two std::string,a boost::any(for a double and double*)
  // and a boost::shared_array<int>
  public:
  A(double value1, double value2)
  { 
    val[0] = value1;
    val[1] = value2;
  }
  // with normal constructor stuff here, but also
  A(A* const& a) // a pointer-to-object "copy"-constructor?!
  {
    memcpy(this->val, a->val, sizeof(double) * 2 ); 
    // no delete here either...
  }
}

std::map のように使用されます (ポインターマップではありません!)

typedef std::map<std::string, A> aMap;
aMap mapOfA;

そして、コードのいたるところにこれらの小さな宝石がたくさんあります。

mapOfA.insert(std::make_pair("elite", new A(1.337)));

Aの前の「新しい」に特に注意してください!私の知る限り、キーワード「new」はヒープにメモリを割り当てます。これは、もう一度削除して削除する必要があります(ブーストポインターなどを除く)。今日でも正しいことを願っています。;)

したがって、コードはヒープ上にオブジェクトを作成し、それをその特別なポインター変換コピー コンストラクターにポインターで渡します。これは、マップがポインターではなくオブジェクト全体を格納したいため、それ自体を値で std::make_pair に渡します。これまでのところ、私はこれを正しく理解していますか? 好奇心から、「コピー」コンストラクターについてコメントしたところ、予想どおり、すべての小さな宝石 (A* から非スカラー型 A への変換が要求された) でコンパイラ エラーが発生しました。

それがGCC 4.1.2でコンパイルされているとすれば、これが実際に役立つかどうかわからないメカニズムはありますか? 私の意見では、それは本当に悪い習慣です!遅いだけでなく、大量のメモリ リークも発生しますね。

編集:

実際にはboost::shared_arrayなので、実際には大丈夫なmemcpy機能について多くのコメントをしているpplのため、doubleの配列に変更します。

4

3 に答える 3

1

うん!をドロップしてmemcpy、単に使用してみませんか:

val = a->val;
于 2013-08-21T12:34:15.027 に答える
1

あなたの理解は正しく、このコードは正気ではありません。作者は C++ のメモリ管理も、代入のような基本的な概念も理解していないようです。これmemcpyは奇妙な難読化ですval = a->val;

推測するなら、作成者はnew(他の一般的な言語と同様に) クラス オブジェクトを作成する際に使用する必要があるという誤解があり、その後のコンパイル エラーを修正するために気紛れなコンストラクターを追加したと言えます。

正しい修正は、おそらく削除することnewです:

mapOfA.insert(std::make_pair("elite", A(1.337)));

または1.337、 からの変換doubleが暗黙的であると想定されている場合は、単に 。

于 2013-08-21T12:55:16.453 に答える