C++における、vector<Class Hoge>のインスタンス生成方法

Question

前提・実現したいこと

C++で開発を行っている初心者です。
解決方法がわからないので教えてください。

該当ソースは以下のMakeEquipmentListです。
簡略化しておりますが、
Enchantクラスにprivateで保持しているvector<Equipment> _equipmentListを生成しようとしているメソッドです。

しかし、この状態ではループが終了した時点でスコープ外となり、_equipmentListの中身が開放されてしまいます。
理屈はわかるのですが、
ループやメソッドを抜けたとしてもデータを保持するためには、具体的にどのようにコーディングすれば良いのかがわからないので教えてください。
よろしくおねがいします。

該当のソースコード

void Enchant::MakeEquipmentList() {
	vector<vector<string>> tmp = { { "sword", "10", "20" }, {"armor", "30", "40"} };
	for (auto itr = tmp.begin(); itr != tmp.end(); itr++) {
		vector<string> tmp = (*itr);
		Equipment *equip = new Equipment(&tmp);
		_equipmentList.push_back(*equip);
	}
}

class Enchant {
public:
    MakeEquipmentList();
private:
    std::vector<Equipment> _equipmentList;
}

Answer 1

こんにちは。

この状態ではループが終了した時点でスコープ外となり、_equipmentListの中身が開放されてしまいます。

Equipmentクラスの実装次第ですよ。Equipment(std::vector<std::string>* tmp)コンストラクタがtmpをコピーして保持すれば大丈夫なはずです。tmpのポインタや参照で保持するとダメです。

#include <iostream>
#include <string>
#include <vector>

using namespace std;

class Equipment
{
    std::vector<std::string> data;
public:
    Equipment(std::vector<std::string>* tmp) : data(*tmp) { }
    void print() const
    {
        for (auto const& item : data)
        {
            std::cout << item << " ";
        }
        std::cout << "\n";
    }
};

class Enchant {
public:
    void MakeEquipmentList();
    void print() const
    {
        for (Equipment const& equipment : _equipmentList)
        {
            equipment.print();
        }
    }
private:
    std::vector<Equipment> _equipmentList;
};

void Enchant::MakeEquipmentList() {
    vector<vector<string>> tmp = { { "sword", "10", "20" }, {"armor", "30", "40"} };
    for (auto itr = tmp.begin(); itr != tmp.end(); itr++) {
        vector<string> tmp = (*itr);
        Equipment *equip = new Equipment(&tmp);
        _equipmentList.push_back(*equip);
    }
}

int main()
{
    Enchant enchant;
    enchant.MakeEquipmentList();
    enchant.print();
}

更に、C++11以降であれば、std::initializer_listを受け取るstd::vectorのコンストラクタ（左のリンク先の「例」参照）を使うとMakeEquipmentList()をよりスッキリ記述できます。

---

【ミスが会ったので修正】

std::initializer_listを受け取るstd::vectorのコンストラクタ（左のリンク先の「例」参照）を使うと

の記載はミスでした。以下へ訂正します。

Equepmentにstd::initializer_listを受け取るコンストラクタを定義してstd::vecotrへ渡す（cppreffpの例参照）と

Comments

[Tatatatatana]

回答有り難うございます。

解決しました！
例で示してくださったEquipmentクラスのdataが、僕の作成したものではポインタとなっていたことが原因でした。
コンストラクタにて値コピーをするよう変更しました。

[Tatatatatana]

std::initializer_listについても初耳でしたので助かります。勉強になりました。

[Chironian]

うまく行ってよかったです。
ところで、私の回答に一部ミスがありました。修正しますね。

[Tatatatatana]

意味は伝わっておりました。ありがとうございます。

Answer 2

あまり詳しくないですが、
保持する方をポインタにしてはいかがですか?
最近の方法であればshared_pointerとかのスマートポインタっていうやつ。

つまり、私が言いたいのは、仮に生のポインタだとしたら、

class Enchant {
public:
    MakeEquipmentList();
private:
    std::vector<Equipment> _equipmentList; // <- これ!
}

を、ポインタを保持するようにして、

class Enchant {
public:
    MakeEquipmentList();
private:
    std::vector<Equipment*> _equipmentList; // <- これ!
}

とする。この場合は デストラクタで vector内にあるオブジェクトをdeleteしないといけませんが、
これなら保持できる。

私の考えでは(※ 正しいわけではないことに注意) MakeEquipmentListメンバ関数内ではEquipmentをnewで生成している。

そして、その実体をメンバ変数として保持していますね。

だから関数から離れた瞬間に破棄されるのでは?

(趣味でやっているのでうのみにはしないでください...)

Comments

[Tatatatatana]

回答ありがとうございます。
定義をご指摘のとおりvector<Equipment*> _equipmentListに修正し、ループの中を以下のようにしましたが、結果は変わらずでした。
何か他に改善点など思いつきますでしょうか。

vector<string> tmp = (*itr);
_equipmentList.push_back(new Equipment(&tmp));

[Tatatatatana]

解決しました。ありがとうございました。

Answer 3

これで問題ないのではないでしょうか。

_equipmentList.push_back(Equipment(&tmp));

下手にポインタを介するから煩雑になるわけで、push_backはconst T&からのコピー、あるいはT&&からのムーブで格納されるので（cpprefjp）、そのまま一時オブジェクトを渡して問題ありません。

Comments

[maisumakun]

元のコードの場合も、vectorにはnewしたオブジェクトのコピーが入るため、動くことは動きます（逆に。newしたオブジェクトをdeleteしていないので、そちらがメモリリークするほうが問題になります）。

[Tatatatatana]

回答有り難うございます。
一時変数を撤去し以下のように修正しましたが、結果は変わらずでした。
何か他に改善点など思いつきますでしょうか。

vector<string> tmp = (*itr);
_equipmentList.push_back(*new Equipment(&tmp));

[Tatatatatana]

解決しました。ありがとうございました。