- PR -

C# インデックスが配列の境界外です

投稿者投稿内容
Jitta
ぬし
会議室デビュー日: 2002/07/05
投稿数: 6267
お住まい・勤務地: 兵庫県・海手
投稿日時: 2005-11-24 19:22
引用:

AKIRさんの書き込み(2005-11-24 10:08)より:


 まず、ソースの“一部”を掲載されても、間違いを指摘するのは難しいです。なぜなら、プログラムを作るときは、

「このように動いて欲しい」という要望があり、
それをコードという形で表します。

この、「要望」と「コード」にズレがあれば、「おかしな動き」をすることになります。ソースを提供されても、「要望」が提供されなければ、直すべき「ズレ」に到達することは出来ません。

具体的に説明すると、、、
 『zi++としてるところでエラーになる(かな?)かもしれないような気がする』とのことですが、この zi という変数を、どういう目的で用意したのか、わかりません。変数名からも、推測することが出来ません。その為、これがおかしいのかどうか、私にはわかりません。
 確かに、この zi が悪さをしています。しかしそれはインクリメントしているからではなく、クラス変数として宣言し、メソッドに入ったときに初期化(0にリセット)されないからです。しかし、この動作が正しいのかどうかは、私にはわかりません。どういう意図を持ってこの変数を用意したのか、わからないからです。
 同じような箇所を探せば、k も悪さをするでしょう。しかしこっちの場合、for 文で 100 以下の時しか参照されていません。したがって、エラーにはなりません。しかし、for 文の中を通らないという、気づきにくいバグとなっています。
 もっとも、この指摘が正しいのかどうかはわかりません。どの様な意図で変数が用意され、コードが書かれたのか、わからないからです。


 最初の投稿では、 sortPlayer というメソッドでした。しかし今回は、Titya というメソッドに変わっています。やはり、この名称からも、このメソッドが何をするのか、わかりません。
 ti に乱数が入り、以前のもの同じであればもう一度乱数を発生させているところから、1〜4のプレイヤーの順番を、不定な順に並べ直したいのだと思います。しかし、それにしても int 配列が1つあれば足りるはずで、yi や zi、k がどんな役割をするのか、わかりません。
コード:
for (int outerIdx = 0; outerIdx < 4; idx++) {
	int order = -1;
	while (order < 0) {
		order = r.Next(4);
		// ここの判定は、メソッドを分けるべき
		for (int innerIdx = 0; innerIdx < outerIdx; innerIdx++) {
			if (ti[innerIdx] == order) {
				order = -1;
				break;
			}
		}
	}
	ti[outerIdx] = order;
}


 とりあえず、順番のシャッフルだとすると、任意のプレイヤー2人を取り出して、順番を入れ替えます。
コード:
ランダムに選んだ○のところを入れ替えることを、任意の回数繰り返す
→実行の繰り返し→→
1○  3○  4   4   4
2   2   2○  3○  1
3○  1   1   1○  3
4   4○  3○  2   2

おおよそ、Length の半分繰り返せば、シャッフルされることが期待できる。
不安なら、片方を上から順番として、Length 回繰り返せばよい。

// 未検証
// 入力 players をシャッフルした、複製された配列を返す
public int[] ShufflePlayers(int[] players) {
	int[] returnValue = new int[players.Length];
	for (int idx = 0; idx < players.Length; idx++) {
		returnValue[idx] = players[idx];
	}
	Random rand = new Random((int)DateTime.Now.Ticks);
	for (int count = 0; count < players.Length; count++) {
		int idx1 = rand.Next(players.Length);
		int idx2;
		do {
			idx2 = rand.Next(players.Length);
		} while (idx1 == idx2);
		int work = returnValue[idx2];
		returnValue[idx2] = returnValue[idx1];
		returnValue[idx2] = work;
	}
	return returnValue;




引用:

でも実装する前のTest段階のときはこんなエラーでませんでした。また、繰り返しますがコンストラクタに移してからエラーはでません。


 コンストラクタに移してからエラーは発生しないということですが、コンストラクタと OnPaint が、どの様なときに実行されるかを理解する必要があります。
 コンストラクタは、オブジェクトが生成されるとき、一回だけしか実行されません。しかし、OnPaint は、コントロールを描画しようとするときに実行されるので、プログラムの実行中に何度も実行されます。何度も実行されるので、zi は増える一方となり、3 を超えた時点で例外が発生したのでしょう。また乱数を用いていますから、結果も乱数に依存することになります。デバッグの時は固定のシードを与え、固定の乱数が出るようにしましょう。

 プログラムは、作った人の意図の通りではなく、作られたとおりに動きます。作られた結果が意図と違っていれば、「おかしな動き」をします。作られた結果とあなたの意図が、どこでずれているのか、検証する必要があります。

 コンストラクタに移したら大丈夫だ。きちんとした意図を持ってそうしたのなら、それでかまいません。しかし、適当に「コンストラクタに移してみたら?」と思ってやったらうまくいった、ということなら、危険です。sort という名前から、任意の時に任意の回数実行されなければならない、と思います。本当にコンストラクタに移すのが正しいのですか?
___________________________________________________________________
□ written by Jitta on 2005/11/24
□ Microsoft MVP for Visual Developer ASP/ASP.NET Oct.2005-Sept.2006
# じゃんぬさん:
# 揚げ足取りみたいな指摘になって済みません。
_________________
AKIR
常連さん
会議室デビュー日: 2005/11/08
投稿数: 34
投稿日時: 2005-11-24 21:23
引用:
コンストラクタは、オブジェクトが生成されるとき、一回だけしか実行されません。しかし、OnPaint は、コントロールを描画しようとするときに実行されるので、プログラムの実行中に何度も実行されます。


コンストラクタに移したのは直感でした。教えていただきありがとうございます。
はじめのうちは、TityaのなかでsortpにplayerをいれるようにしていてTityaをOnPaint
の中で呼び出してたんです。
ランダムにダブりなく数字を並べたかったので、考えたらこういうコードになってし
まいました。zi=0;をメソッドの中に入れてやればよかったのですね。
今、習作として麻雀ゲームを作っているのですが、Tityaとは起家(ちーちゃ)最初の親
と卓を囲むplayerの配置を決めるメソッドですので、1回でいいのです。
ランダムシャッフルってswapとかいうの使ってるの検索したらヒットしましたが、
Jittaさんがいったようなことをするものだったんですね。知りませんでした。
僕は、乱数発生させて、それをforで囲んでやって、でもそれだとダブったりするから
次のforで同じもでないか==でチェックしてもし同じだったら新たに乱数発生させて
で違うものだったらti[i]に入れるようにさせました。でも、これだけだとiより前のもとは比較できないのでダブる可能性があります。そこでinitj:にジャンプさせました。
ziは少しでもkの値を少なくするようにしようとして、iが2を超えたらziを0から1づつ足していくようにしてti[3],ti[4]に入るようにしました。
136ある牌も同じようにしているのでyiを600000用意して、kがだいたい20万から40万
くらいになってました。遅いときで4秒はかかります。
Jitta
ぬし
会議室デビュー日: 2002/07/05
投稿数: 6267
お住まい・勤務地: 兵庫県・海手
投稿日時: 2005-11-25 06:38
int work = returnValue[idx2];
returnValue[idx2] = returnValue[idx1];
returnValue[idx2] = work;

この部分が swap です。
_________________

スキルアップ/キャリアアップ(JOB@IT)