3

これは不可解です。プログラムで CCountry::getName() 関数を使用する必要があります。奇妙なことに、それがまったく機能するかどうかをテストすると、1 つの場所では機能しますが、2 行下では機能せず、その理由がわかりません。例えば...

while(line != "---" && line != "------")
    {
        CCountry *tempCountry = new CCountry(line);
        cout << tempCountry->getName() << flush;
        (*tempContinent).addCountry(*tempCountry);
        getline(filestr, line);

    }

動作します。すべての国名を順番にリストします。でも...

    while(line != "---" && line != "------")
    {
        CCountry *tempCountry = new CCountry(line);
        (*tempContinent).addCountry(*tempCountry);
        getline(filestr, line);
        cout << tempCountry->getName() << flush;
    }

動作しません。getName() を呼び出す行でセグ フォールトをスローする代わりに、国名を 1 つでも出力できません。

さらに参照するために、getName() と addCountry() の 2 つの関数を次に示します。

string CCountry::getName()
{
return *name;
}

と

void CContinent::addCountry(CCountry country)
{
(*countries).push_back(country);
}

リクエストごとに、CCountry コンストラクターは次のとおりです。

CCountry::CCountry(string in_name)
{
name = new string;
*name = in_name;
player = new int;
*player = -1;
units = new int;
*units = 0;
neighbors = new list<CCountry>;
}
4

5 に答える 5

3

このコードの問題点の長いリストをガタガタ鳴らすことができますが、最終的にあなたの過ちを引き起こしているのは、次の理由によるものです。

あなたの CCountry クラスは、動的に割り当てられたメンバーを持っているため、3 のルールを実践していません。（ところで、これは必要ありません）。

国を値で受け取るメンバー関数を介して、CCounty オブジェクトを大陸に追加しています。その時点で、オブジェクトの浅いコピーが作成されます。次に、これを大陸内のコンテナーにプッシュすると、別の浅いコピーが作成されます。addCountry() の終了時に元の浅いコピーが破棄され、そのプロセスで呼び出し元のコードに戻るまでに、そこにある CCountry オブジェクトの内部が破棄されています。したがって、あなたのローカル（最初に動的に割り当てられるべきではありませんでした）は、公式にホースされています。

そして、何を推測しますか...あなたの大陸コンテナの中のものもそうです。

CCountry オブジェクト自体について考えることから始めたいと思います。個人的には、CCountry オブジェクトのコレクションがとにかく管理される場所であるため、CCountry ではなく CCountry クラスで CCountry の隣人を管理しますが、それぞれ独自のものです。現在のモデルに固執することにした場合、CCountry の潜在的な代替案は次のようなものになる可能性があります。

class CCountry
{
public:
    CCountry(const std::string& name)
       : name(name), player(0), units(0)
    {
    }

    // properties
    const std::string& getName() const { return name; };
    int getPlayer() const { return player; };
    void setPlayer(int player) { this->player = player; };
    int getUnits() const { return units; };
    void setUnits(int units) { this->units = units; };

    // neighbor access
    const std::list<const CCountry*> getNeighbors() const
    {
        std::list<const CCountry*> res;
        for (auto it=neighbors.begin(); it != neighbors.end(); ++it)
            res.push_back(it->second);
        return res;
    }

    // adding a new neighbor
    void addNeighbor(const CCountry& other)
    {
        neighbors[ other.getName() ] = &other;
    }

private:
    std::string name;
    int player;
    int units;
    std::map<std::string, const CCountry*> neighbors;
};

ただし、注意してください: このようなモデル (および、ご覧のとおり、元のモデル) を追求すると、潜在的な落とし穴があります。具体的には、CCountry が、技術的に所有していない別の CCountry へのポインターを持つ可能性があります 。これが、CCountry とその近隣アソシエーションの両方を所有するため、CContinent クラス自体によって近隣アソシエーションを管理することを好む理由です。

于 2012-12-29T23:00:40.980 に答える
2

CCountry次のようなデストラクタを定義したと思われます。

~CCountry() {
    delete name;
    delete player;
    delete units;
    delete neighbors;
}

しかし、 のコピー コンストラクターを定義していないと思われますCCountry。これは、コンパイラが次のようなコピー コンストラクタを生成していることを意味します。

CCountry(CCountry const &that) :
    name(that.name),
    player(that.player),
    units(that.units),
    neighbors(that.neightbors)
{ }

現在、はではなくCContinent::addCountryを取るように定義されています。したがって、 を実行すると、プログラムはそのコンパイラ定義のコピー コンストラクターを使用しての (一時的な) コピーを作成します。CCountryCCountry &(*tempContinent).addCountry(*tempCountry)*tempCountryCCountry

したがって、プログラムには の 2 つの個別のインスタンスCCountryがありtempCountryます。ただし、コンパイラ定義のコピー コンストラクターの動作方法により、両方のインスタンスが同じインスタンスを指すメンバー変数を持ちます。CContinent::addCountrycountrynamestring

一時コピーが削除されると、そのデストラクタがその文字列インスタンスを削除します。が指すインスタンスのメンバ変数tempCountryにダングリング ポインタが含まれるようになりました。nameでぶら下がっているポインターを逆参照しようとするとgetName、動作が未定義になり、セグメンテーション違反が発生します。

、、、およびメンバー変数をポインターにならないnameように変更します。次のように、単純な型にする必要があります。playerunitsneighbors

class CCountry {
    string name;
    int player;
    int units;
    list<CCountry *> neighbors;
};

また、コピーではなく参照を取るように関数を変更することもできます。

于 2012-12-29T23:01:54.377 に答える
0

CCountry のコンストラクターでは name を割り当てnew string、デストラクタではおそらく で解放しdelete nameます。なぜこれを行う必要があるのか​​ わかりません。 のメンバーとして保存するよりも簡単に保存できる場合stringがstring*ありnameますCCountry。CCountryの一時コピーに引数として渡すとCContinent::addCountry、その一時コピーが作成されてから削除され、その削除がCCountry::nameの複数のインスタンス間で共有されますCCountry。これを回避する には、CCountry のメンバーとしての代わりstringに使用するか、独自のコピー コンストラクターを実装する必要があります。string*nameCCountry

于 2012-12-29T23:05:28.457 に答える
0

CCountry のコンストラクターで何らかの上書きを引き起こした可能性はありますか? 私のように聞こえます。

于 2012-12-29T22:51:52.217 に答える
0

このコードには多くの問題がありますが、まず、CCountry値のセマンティクスがあるか、それともエンティティ型ですか。最初のケースでは、それへのポインタを持ったり、 を使用して動的に割り当てたりするべきではありませんnew。また、正しくコピーして割り当てることができることを確認する必要があります。2 つ目は、 に値渡しするべきではありませんCContinent::addCountry(また、コピー コンストラクターと代入演算子を非公開にするか、 から派生させることにより、コピーと代入を禁止する必要がありますboost::noncopyable)。

の定義は示していませんCCountryが、初期化する方法は、それがエンティティ オブジェクトであるとname想定していることを示唆してい ます。std::stringそうではありません—値のセマンティクスがあり、へのポインターがある場合はほとんどありませんstd;:string。(1 つの例外は、値の不在を示すためにヌル ポインターをサポートする関数のパラメーターまたは戻り値です。) 同じことが、player、unitsおよび: へのポインターまたはへのneighborsポインターを持つコンテキストにも当てはまります。int標準コンテナは、値が存在しないことを示すためにヌル ポインタが必要な場合に限定されます。

また、コピー コンストラクター、代入演算子、またはデストラクターの使用も示していません。デストラクタでメモリを削除していて、コピー コンストラクタがない場合、これが問題の原因です。コンパイラによって生成されたコピー コンストラクターは浅いコピーを行います。つまり、 を呼び出す CContinent::addCountryと、同じポインターを持つ 2 つのオブジェクトが作成されます。引数が破壊されたときに何かを削除すると、引数として渡されたオブジェクト (同じポインターを含む) が無効になります。これを処理するにはさまざまな方法がありますが、ほとんどの場合、最も適切なのはポインターを使用しないことです。std::string(たとえば、クラスには、ディープ コピーを行うコピー コンストラクターがあるため、それを使用しても問題ありません。)

最後に、まったく関係のない問題について: クラス名の先頭に を付けることはおC勧めできません。Microsoft はクラス名にこの規則を採用しています (少なくとも一部のライブラリでは)。このような名前を見た読者CCountryは、それが Microsoft ライブラリの 1 つからのクラスであると想定し、Microsoft のドキュメントでそれを見つけようとします。 、あなたのコードではありません。

于 2012-12-29T23:09:02.760 に答える