重複なしでランダムに画像を表示する方法

質問

現在、C#で重複なしでランダムに画像を表示するプログラムを作成しています。
下記のように書いてコンパイラをすると
型 'string' を型 'int' に暗黙的に変換できません
名前 'n' は現在のコンテキスト内に存在しません。
名前 'n' は現在のコンテキスト内に存在しません。
名前 'n' は現在のコンテキスト内に存在しません。
というエラーが出て直し方が分からない状態です。
このエラーは、GetNextIndex()関数内で起こっています。
どうか力を貸してください。お願いします。

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace WindowsFormsApplication1
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
}

//定数
private const int IMAGES = 3;//イメージの総数=各配列の要素数

//メンバー変数
private string[] sPathes = {@"D:\A.BMP",//１番目の画像のパス
@"D:\B.BMP",//２番目の画像のパス
@"D:\C.BMP",//３番目の画像のパス;//パス配列の宣言と定義
};
private Image[] myImages = null;//イメージ配列変数の宣言

private int m_nIndex;//表示する画像の番号
//private int[] m_nUsed = null;//表示済画像の番号の配列
private int m_nTimes = 0;//これまでに表示した枚数（３まで）

public void Init()//引数なし、メンバー変数の初期化
{
//イメージ配列の初期化
//・・・インスタンスメモリの確保（メモリーの確保は最初の１回だけ）
if (myImages == null)
myImages = new Image[IMAGES];//本当はnewのチェックが必要

//・・・ファイルを読み込んでイメージ配列に格納
for (int n = 0; n < IMAGES; n++)
{
string sPath = sPathes[n];//パス文字列
Image img = Image.FromFile(sPath);//イメージ読込
myImages[n] = img;//配列に格納
}//for終わり
//this.m_nUsed(-1);
}

protected virtual void OnCreate()//OnLoadから呼ばれる
{
//変数の初期化を行う
this.Init();
}

//初回更新用の関数
protected virtual void OnInitialUpdate()
{
this.OnUpdate();//更新をかける
}

//更新関数
protected virtual void OnUpdate()
{
if(m_nIndex < 0)//画像番号が設定されていなければ何もしない。
return;

//画像の更新
pictureBox1.Image = myImages[m_nIndex];
}

//次に表示する画像番号の取得
protected int GetNextIndex()
{
//ランダムな配列を作成
Random r = new Random();
//重複チェック
int[] a = new int[n];
for (int i = 0; i < n; ++i) a[i] = i + 1;
for (int i = n; i > 1; --i)
{
int k = r.Next(i);
int tmp = a[i - 1];
a[i - 1] = a[k];
a[k] = tmp;
}
return r.ToString();
}

private void button1_Click(object sender, EventArgs e)
{
if(m_nTimes >= IMAGES)
return;
int n = GetNextIndex();//次なる番号の取得
if (n < 0)//適当な番号が得られなければ
return;//何もしない

//やっと画像番号を変える
m_nIndex = n;

//更新を呼び出す
this.OnUpdate();

}
}
}

回答No.2

kmee · ベストアンサー

> 値として何を返せばよろしいのでしょうか？

それを考えるのが「プログラミング」というものです。

まず、プログラムの仕様をまとめた方がいいです。
それから、一気に作らず、各メソッドが期待通りに動作しているか、テストした方がよいでしょう。今回のだったら、画像の表示等は後にして、GetNextIndexが「重複していない整数」をちゃんと返すかどうか、確認してから次へ進めましょう。

button1_Clickの動作からして、GetNextIndexは、m_nIndexに設定するための0以上IMAGES未満の整数を、重複無しにランダムに返してくることが期待されているのではないでしょうか。あとは、エラー時には負の整数を返すことも。

そこで、GetNextIndexを見ると、returnの直前では
int a[] : 1以上n以下の整数をシャフルした配列
となっています。nは定義されていないので、IMAGESの間違いで、
> for (int i = 0; i < IMAGES; ++i) a[i] = i + 1;
の a[i] = i + 1;をa[i] = i ;にすれば
int a[] : 0以上IMAGES未満の整数をシャフルした配列
となり、このaの値をGetNextIndexが呼ばれる度に一つずつ返せば、目的の動作になるはずだ、ということがわかります。
なので
return a[X] ;
のような形になるでしょう。

ところが、今のままでは、
・配列aを毎回作っている。これではシャフルの意味がない
・どこまで使って、次に何を返せばいいのか、覚えておく仕組みが無い
・配列をリセットする手段が無い。
と言う問題があります。

解決策としては、Randomのようなインターフェースで専用のクラスを用意するのがいいのでは、と思います。

お礼

回答ありがとうございました。
kmeeさんのおっしゃる通りに１つずつ作成していったら
目標とする動きが出来ました。
ありがとうございました。
それでは、失礼いたします。

回答No.1

kmee

> protected int GetNextIndex()

と宣言したのに

> return r.ToString();

なぜstringで返すのでしょう?
戻り値がintのはずなのに、stringが指定してあって、戻り値の型にあわせるためにstring→intの変換をしようにも変換方法がわからない(暗黙の変換が無い)というエラーです。

そもそもこのGetNextIndexですが
・Randomクラスであるrを返すこと自体おかしい
・呼ばれるたびに、毎回新しい乱数と新しい重複チェック配列を作っています。これらはreturn後に自動で削除されます。
まったく、重複チェックになっていません。

お礼

回答ありがとうございます。
値として何を返せばよろしいのでしょうか？