std:;vector<> std::arrayという二重のイテレーター二重for文がアクセスエラーとなる原因が知りたい。

Question

提示コードの////コメント文の二重for文のコードですがなぜアクセスエラーが発生するのでしょうか？getBlock()はベクターを返す関数でその１ベクターの中がstd::arrayななっていて、二重for文のより全部を走査する文なのですが何が悪いのでしょうか？どうしても原因がわかりません

void Scene::Update()
{
	stage->Update();
	///////////////////////////////////////////////////////////////////////////////////////////////////////////////
	for (std::vector<Block>::iterator itr = stage->getBlock().begin(); itr != stage->getBlock().end(); itr++)
	{
		
	//	for (std::array<MapChip, 4>::iterator it = itr->getMapChip().begin(); it != itr->getMapChip().end(); it++)
	//	{

		//}
		
	}	
	///////////////////////////////////////////////////////////////////////////////////////////////////////////////
	
}

std::vector<Block> Stage::getBlock()
{
	return block;
}

class Entry;
class Block : public Object
{
public:

	Block(Entry* e, ObjectType type, std::shared_ptr<TextureData> data, TextureUV uv, glm::ivec2 pos);
	Block(const Block& b);	
	~Block();


	std::array<MapChip, 4> getMapChip();

	void Update();
	void Draw();

private:

	std::array<MapChip,4> mChip;

};

Answer 1

質問コードで起きていることを示すために簡単なコードを示す．

class TestData
{
public:
    ~TestData(){    std::cout << "TestData::dtor\n";    }
    std::vector<int> Vec{1,2};
};

class DataOwner
{
public:
    TestData GetData(){ std::cout << "GetData()\n"; return m_Data;  }
    TestData m_Data;
};

int main(void)
{
    DataOwner Owner;
    for( std::vector<int>::iterator iter=Owner.GetData().Vec.begin(); true; )
    {
        std::cout << "Do something with iter" << std::endl;
        break;
    }
    return 0;
}

↑のコードにて，
for( std::vector<int>::iterator iter=Owner.GetData().Vec.begin();
の部分で Owner.GetData() の戻り値として返されてきた TestData 型オブジェクトというのは，果たしていつまで生存しているのであろうか？　という話だ．

実行してみれば，
"Do something with iter"
よりも前に，
"TestData::dtor"
が表示されることが確認できるだろう．

forのブロック内の処理を行おうとした時，iter が指す先のオブジェクトは既に解体されている．

質問者のコード：
for (std::vector<Block>::iterator itr = stage->getBlock().begin();
でも同じことが起きている．

Comments

[fana]

一時オブジェクトの寿命の話は，何日か前にもあったハズ．
https://teratail.com/questions/318818

[退会済みユーザー]

ではどうすればいいのでしょか？イテレーターでは出来ないということでしょうか？

[fana]

> なぜアクセスエラーが発生するのでしょうか？

に関して，その発生要因を述べた．

どうすれば良いのかについては，既に回答がなされていてBAになっているではないか．
範囲forで書くなり，一旦そのスコープの変数に受けてそれを使うなりすればよい．

個人的には，getXXX() とかいうメソッド群を const参照を返す形にするのでも良いのではないかと思うが，
それ以前に getXXX() を上位側が多重に呼んで処理するような形態自体に疑問を感じる．

[fana]

getXXX()を持ってるクラス群の役割って何だろう？
上位側で全データを引き出して処理する形態なのであれば，そいつらは単なる構造体で良いのでは？
（メソッドの戻り値としてデータを引き出すがためにコピーが発生するだの何だのに苦しむくらいなら，もうダイレクトに構造体のメンバをいじくる方が楽なのでは？）

[fana]

> イテレーターでは出来ないということでしょうか？

int32_t氏による前の質問の回答：
https://teratail.com/questions/320459#reply-445309
でイテレータでループする形の書き方も示されているよね．

Answer 2

前の質問の回答、読みましたか?

ついでに言えば、stage->getBlock() で返される vector は毎回違うインスタンスなので、このループは思うように動きません。

---

コード例：

for (auto& block : stage->getBlock()) {
  for (auto& map_chip : block.getMapChip()) {
    ... = map_chip.getCollision();
  }
}

Comments

[退会済みユーザー]

下にある階層のコンテナを拾いたいのですがどうすればいいのでしょうか？

[int32_t]

指摘した点を直してふつうに2重forループすればいいと思いますよ。

[int32_t]

ただ、getBlock() のループは Stage::Update() の中に移動、getMapChip() のループは Block クラスのメソッドに移動、としたほうが良い設計のような気配がします。
(全体のコードを見ていないので的外れかもしれませんが)

[退会済みユーザー]

なるほど。質問ですがこのコードは正解？でしょうか？

for (int i = 0; i < stage->getBlock().size(); i++)
{
for (int j = 0; stage->getBlock().at(i).getMapChip().size(); j++)
{
stage->getBlock().at(i).getMapChip().at(j).getCollision();

}
}

[fana]

それであなたの思うように動くなら「正解」なのだろうし，違うならば「正解じゃない」．
getBlock()だのgetMapChip()だのが何度となく呼び出されてその度にデータのコピーが生成されてくる動作が本当に必要なのかもしれないし，違うのかもしれない．
正確なところは他者には判断不能．

（既にコメントされているように）そのコードを書くこと自体の是非みたいなところから考えた方が良いのかもしれない．

[int32_t]

13:17のコメントのコード、vectorのコピーが無駄に発生しますがそれ以外は問題は見当たりません。
私の前の回答で無駄なコピーの少ないforの書き方も書いてあるんですが、それを参考にしないのですね。

[fana]

> for (int i = 0; i < stage->getBlock().size(); i++)

こういうの書いたら，
「 stage->getBlock() が何回呼び出されることになるのだろうか？」
とか……　気にならないのかなぁ？

[退会済みユーザー]

質問ですがこの二重ループのコードがアクセスエラーを起こします。どうやら中側の二重目のループみたいなのですがどうすればいいのでしょうか？Blockの要素が取得出来なのでその中のstd::array<>を走査したいのですが　追記　回答欄に追記されました。

// プレイヤーと ObjectType::Block ブロックの当たり判定
Box_Collision_2D b = player->getCollision();
auto blocks = stage->getBlock();
for (auto itr = blocks.begin(); itr != blocks.end(); itr++)
{
for (auto it = itr->getMapChip().begin(); it != itr->getMapChip().end(); it++)
{
Box_Collision_2D a = it->getCollision();

if (Intersect_Rect_2D(b, a) == true)
{
//player->FixPos(a.getPosition());
printf("Collision !\n");
}
}
}

[int32_t]

Block::getMapChip() のコードは開示されていませんが、きっと getBlock() とまったく同じ問題でしょう。getMapChip() が呼ぶたびに新しいstd::arrayインスタンスを返していると、イテレータの比較がうまくいかなくて、範囲外アクセスを起こしたりします。

[退会済みユーザー]

getMapChip();はstd::array<>なのですがどうすればいいのでしょうか？

[int32_t]

std::vectorのときと対処は同じです。回答に書いたようなコードで動くはずです。

[退会済みユーザー]

なるほど、コンパイルは出来ましたがアクセスエラーになるのですがなぜでしょうか？

for (std::array<MapChip,4>::iterator it = itr->getMapChip().begin(); it != itr->getMapChip().end(); it++)
{
Box_Collision_2D a = it->getCollision();

if (Intersect_Rect_2D(b, a) == true)
{
//player->FixPos(a.getPosition());
printf("Collision !\n");
}
}

[fana]

（なんだろう，回答された書き方は意地でも採用しないぞ，という強い意志のようなものを感じる）

[int32_t]

この回答のコード読みましたか? for の () の中で getMapChip() を2回以上呼ぶのはダメなんですよ。

[退会済みユーザー]

あっ。ｗなるほど。すいませんでした。ぼーっとしてました。
イテレーターを使ってやってみたかったので