文字列をランダムで作る関数が同じ値を返す

Question

//ランダムな文字列を作成
    private const string CHARS = "0123456789abcdefghijklmnopqrstuvwxyz";
    public static string GenerateRandomNumber(int length)
    {
        System.Text.StringBuilder sb = null;
        sb = new System.Text.StringBuilder(length);
        System.Random r = null;
        r = new System.Random();

        for (int i = 0; i < length; i++)
        {
            int pos = r.Next(VillagerID_CHARS.Length);
            char c = CHARS[pos];
            sb.Append(c);
        }

        Debug.Log(sb);
        return sb.ToString();
    }

こちらの関数を使ってランダムな文字列を返そうとしているのですが、
この処理をfor文で呼び出すと必ず同じ文字列が2つ返されます。

//呼び出しコード
void Call(){
    List<string> str = new List<string>();
    for (int i = 0; i < 4; i++)
    {
        str[i] = GenerateRandomNumber(5);
        Debug.Log(str[i]);
    }
}

デバッグ結果(1回目)↓

5j2kv

3vk54
4kco8
4kco8

〃(2回目)↓

9l654

lfm34
lfm34
7ght0

など重複するタイミングも毎回違います。

なぜ重複してしまうのでしょうか。何度もコードを見直したのですが見つからなかったので質問しました。
どなたか回答お願いします。

Answer 1

毎回乱数生成機を r = new System.Random();と生成している

引数なしだとシードを時刻で作る
時刻で作るということは十分高速で処理が走るとシード値が同じになる

シード値が同じだと同じ結果になる

というわけでRandomクラスのインスタンスをメンバで持って一回だけ初期化しましょう

Comments

[_Beginner]

コンピュータの乱数は時刻をもとに計算されているのは知っていたのですが、
そのせいでこうなっていたのですね。
参考になりました。ありがとうございます！

Answer 2

System.Random()は、時刻相当をベースに初期化するので、短い間隔で複数回呼ぶと同じ乱数系列になります。
System.Random()は一度だけ呼んで、インスタンスを保存して使いまわすとよいかと思います。
あるいは、毎回異なる引数で呼び出すか。
あるいは、ミリ秒単位でスリープを入れるか。

Comments

[_Beginner]

異なる引数、スリープ、という手もあるのですね。
今回は初期化を一回だけすることにします。ありがとうございます！

Answer 3

確認してませんが、、、

    System.Random r = null;
    r = new System.Random();

関数の中でnewしているので、タイミング次第で、同じ初期値では?
グローバルで、初期化(new)するか、呼び出し側で、初期化し、それを使用するすれば、どうでしょう。

Comments

[_Beginner]

なるほど。
グローバルで初期化することにしました。ありがとうございました。

Answer 4

r = new System.Random();

new を行うことにより、乱数発生ロジックは初期化され、以降、同じ数列を出すことになります。

ということで、その関数の中でnewせず、関数の外で、スタティック変数にしてしまいましょう

それでも、そのプログラム自体の起動時点から同じ文字列が出ることになりますんで、プログラム開始時点で現在日時を取得し、それをもとにnextをループさせましょう

Comments

[Q71]

補足
乱数ジェネレーターは、アプリケーションで1つだけ作るようにしましょう。また、スレッドセーフではないので、注意しましょう。詳しいことは、リファレンス（機械翻訳が読みにくいですが）に書いてあります。

[_Beginner]

素早く、わかりやすい回答ありがとうございます！
一番早かったためベストアンサーに選びます。

>>Q71さん
補足ありがとうございます。
こういうものは使いまわしたほうがいいのですね。ありがとうございます。

[Zuishin]

ベストアンサーですが、間違っている、もしくは誤解を招く表現なので低評価しました。
new System.Random() は現在時刻をシードにしますから、最後の一文は冗長です。