Fisher–Yates shuffle in C#

Question

I have this method Shuffle that supposes to return a set of random numbers which was displayed in the above method but the numbers need to be displayed in a mixed format. The first method works well since the number are being displayed correctly in the set of range but this method Shuffle is not returning them in a mixed format.

Example:

The first method returned: 1, 2, 3, 4, 5

This method needs to return 2, 1, 4, 5, 3

    public int[] Shuffle(int[] Sequence)
    {
        int[] Array = new int[Sequence.Length];

        for(int s=0; s < Array.Length-1; s++){
           int GenObj = GenerateAnotherNum (0, Array.Length + 1);

            Array[s] = Sequence[GenObj];
            Sequence[GenObj] = Array[s];
        }
        return Sequence;
    }

It's unclear what you're asking. What is this code ? Is that the first or second method ? Then what is the other method ? Why do you expect your method to return a specific sequence if it is supposed to be shuffled randomly ? — Arthur Attout, May 30 '19 at 12:53
You should be careful naming your variables. The standard way is to name local variables and arguments with an initial lower case (camelCase). That would help avoid collisions with keywords, like `Array`... — Heretic Monkey, May 30 '19 at 12:55
This code is the Fisher–Yates shuffle method. I just gave an example of what the sequence should return. @ArthurAttout — User89, May 30 '19 at 12:55
swap procedure is *incorrect*, it should be `int h = Array[s]; Array[s] = Sequence[GenObj]; Sequence[GenObj] = h;` — Dmitry Bychenko, May 30 '19 at 12:55
`Array[s] = Sequence[GenObj]; Sequence[GenObj] = Array[s];` is not doing anything. You need to swap the value. — Gilles-Philippe Paillé, May 30 '19 at 12:55
Take a look at this [answer](https://stackoverflow.com/a/1262619/4685428) — Aleks Andreev, May 30 '19 at 12:56
When i do that type of swap it gives me An index out of bound error.. @DmitryBychenko — User89, May 30 '19 at 12:57
@User89: please, have a look at `GenerateAnotherNum` routine. `int GenObj = GenerateAnotherNum (s, Array.Length);` — Dmitry Bychenko, May 30 '19 at 13:02

Dmitry Bychenko · Answer 1 · 2019-05-30T13:45:22.423

You have several problems here: all zeroes array, range and swap procedure

Algorithm:

https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle

Code:

// probably "static" should be added (depends on GenerateAnotherNum routine)
public int[] Shuffle(int[] Sequence)
{
    // public method's arguments validation
    if (null == Sequence)
        throw new ArgumentNullException(nameof(Sequence));

    // No need in Array if you want to modify Sequence

    for(int s = 0; s < Sequence.Length - 1; s++)
    {
        int GenObj = GenerateAnotherNum(s, Sequence.Length); // pleace, note the range

        // swap procedure: note, var h to store initial Sequence[s] value
        var h = Sequence[s];          
        Sequence[s] = Sequence[GenObj];
        Sequence[GenObj] = h;
    }

    return Sequence;
}

Demo:

// Random(0) - we want to reproduce the results in the demo
private static Random random = new Random(0); 

// Let unknown GenerateAnotherNum be a random
private static int GenerateAnotherNum(int from, int to) => random.Next(from, to);

...

int[] array = new int[] { 1, 2, 3, 4, 5 };

string result = string.Join(", ", Shuffle(array));

Console.Write(result);

Outcome:

4, 5, 2, 3, 1

the parameter to get the random number could also be in the range of `0-s` — AD.Net, May 30 '19 at 13:50
Great Answer, Although I already fixed the issue with another given answer this is a good example! — User89, May 30 '19 at 16:08

score 1 · Answer 2 · answered Oct 05 '21 at 17:44

public static class Shuffler<T>
{
    private static Random r = new Random();
   
    public static T[] Shuffle(T[] items)
    {      
        for(int i = 0; i < items.Length - 1; i++)
        {
            int pos = r.Next(i, items.Length); 
            T temp = items[i];          
            items[i] = items[pos];
            items[pos] = temp;
        }
        return items;
    }

    public static IList<T> Shuffle(IList<T> items)
    {      
        for(int i = 0; i < items.Count - 1; i++)
        {
            int pos = r.Next(i, items.Count); 
            T temp = items[i];          
            items[i] = items[pos];
            items[pos] = temp;
        }
        return items;
    }
}

Fisher–Yates shuffle in C#

2 Answers2

Linked

Related