気が付かないバグはバグでない？

１年以上全然気が付きませんでしたorz
や、だって落ちること無かったし違和感なかったし・・・。

std::list< Object* >::iterator it = apObject.begin();
std::list< Object* >::const_iterator const itEnd = apObject.end();

for( ; it != itEnd; ++it ){
	(*it)->NextFrame();
	if( (*it)->IsEnd() ){
		delete *it;
		it = apObject.erase( it );
	}
}

超簡易的なタスクシステムで、仕様はまあ分かると思うけど、全要素のNextFrameを呼び出して、終了している要素は削除していくというもの。これを1/60秒ごとに呼び出される関数の中に埋め込めばシミュレーションの完成(違)
だがこの実装、悪い点が２つある。一つは仕様的なバグ、もう一つは文法的なバグ。ただし文法的バグは別にバグで無い可能性もある。さてどこでしょう？（ぇ

文法的バグの方はendイテレータを変数として持ったこと。これ自体は問題ない。実際、endメソッドをループごとに呼ぶのがパフォーマンス的によくないという理由で変数に代入するというのはよくやる技である。しかしこれは、ループ中にコンテナのサイズが変わらないという条件が必要である。コンテナのサイズが変更された時点でイテレータは意味を成さなくなる(仕様上、保障はされなくなる)。ただ今回の場合はコンテナがlistなのでlistの構造上、endイテレータは変わらない可能性は高い。しかし絶対ともいえないのでこのコードは危険である。

で仕様的バグのほうだが今回のメインはここ。終了している要素の次の要素のNextFrameが呼び出されない。
eraseメソッドの仕様はイテレータを受け取ってそのイテレータの指している要素を削除し、そして削除した要素の次の要素のイテレータを返す。つまり、eraseを呼び出した時点で次の要素に移動しているのだ。なのにforの方で毎回インクリメントしているので、eraseを呼び出された場合要素が二つすすむ。

というわけで、弾幕作ってたら模様がいびつになってはじめて気が付きましたとさorz

一応、修正版

std::list< Object* >::iterator it = apObject.begin();

while( it != apObject.end() ){
	(*it)->NextFrame();
	if( (*it)->IsEnd() ){
		delete *it;
		it = apObject.erase( it );
	}else{
		++it;
	}
}

もしくは

std::list< Object* >::iterator it = apObject.begin();
std::list< Object* >::const_iterator itEnd = apObject.end();

while( it != itEnd ){
	(*it)->NextFrame();
	if( (*it)->IsEnd() ){
		delete *it;
		it = apObject.erase( it );
		itEnd = apObject.end();
	}else{
		++it;
	}
}

おまけ。無理やり１行にしてみました。

for(std::list::iterator it=apObject.begin();it!=apObject.end();(*it)->NextFrame(),(*it)->IsEnd()?delete*it,it=apObject.erase(it):++it);