効果のないアレイシャッフラー

StackOverflow https://stackoverflow.com/questions/4057172

  •  27-09-2019
  •  | 
  •  

質問

私は配列をシャッフルしようとしていますが、私がそれをしている方法は5回目にしかうまくいきません。誰かがそれが適切に機能していない理由を説明し、おそらく微調整を提案してくれたら大いに感謝します。

private Button[] scrambleBoard(Button[] buttons)
{
    for (int x = 100 * buttons.Count(); x > 0; x--)
    {
        Random rand = new Random();
        int first = rand.Next(buttons.Count());
        int second = rand.Next(buttons.Count());


        Button temp = buttons[first];
        buttons[first] = buttons[second];
        buttons[second] = temp;
    }

    return buttons;
}
役に立ちましたか？

解決

次の行をループの外側に移動します。

Random rand = new Random();

使用されるデフォルトの種子 System.Random に基づいています Environment.TickCount. 。タイトなループでは、ティックカウントが連続した反復間で変化しない可能性があるため、同じ種子を何度も使用することになります。その結果、ループは繰り返し交換します 同じ ティックカウントが変更されるまで2つの要素（ループが完了する前にそうしない場合があります）。これが問題であることを確認するために、 Thread.Sleep(100) またはループ内の類似。その後、シャッフルが正しく機能するのを見ることができるはずです（非常にゆっくりとはいえ）。

また、次のことに注意する必要があります 使用しているテクニック 配列を施行するには バイアス;すべての順列が等しくありそうなわけではありません。あなたは、偏りがないことが知られているシャッフルアルゴリズムを使用することをお勧めします。 フィッシャー - シャッフル.

または、本当にシンプルなテクニックを使用してシャッフルできます。それはわずかに非効率的ですが、公平です：

var rand = new Random();
return buttons.OrderBy(button => rand.Next()).ToArray();

他のヒント

問題は、ループの各反復でランダム（）オブジェクトを作成することです。ランダムオブジェクトは初期化中にシードを使用するため、ほとんどの値は同一であり、ランダムではないことがわかります。

ランダムクラスをメソッド本体の外側の静的として宣言することにより、問題を修正できます。

private static Random rand = new Random();

private Button[] scrambleBoard(Button[] buttons)
{
    for (int x = 100 * buttons.Count(); x > 0; x--)
    {
        int first = rand.Next(buttons.Count());
        int second = rand.Next(buttons.Count());


        Button temp = buttons[first];
        buttons[first] = buttons[second];
        buttons[second] = temp;
    }

    return buttons;
}

あなたの質問は答えられましたが、LINQとGUIDを使用してコレクションをシャッフルするための素敵な小さなトリックを共有していると思いました。これにより、値の広がりがあるランダムに順序付けられたリストが作成されます。

private Button[] scrambleBoard(Button[] buttons)
{
    return buttons.OrderBy(b => Guid.NewGuid()).ToArray();
}
ライセンス： CC-BY-SA と 帰属
所属していません StackOverflow
scroll top