Each object have the same array

Question

I have problem with shuffling array in several objects of the same class.
I use function void shuffle() on constructor call.
After printing my dataCharset array it turns out that every object have the same shuffled array.

Im using .net framework 4.8 in console application.
I have tried using a temporary array and then copying it shuflled way into dataCharset array ( which is my targed array i need to shuffle ).

char[] dataCharset =
{
    'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
    'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
    'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
    'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
    '1', '2', '3', '4', '5', '6', '7', '8', '9', '0',
    '!', '$', '#', '@', '-'
};

void shuffle()
{
    Random random = new Random();

    for (int i = 0; i < dataLength; i++)
    {
        int index = random.Next(0, dataLength);
        char temp = dataCharset[index];
        dataCharset[index] = dataCharset[i];
        dataCharset[i] = temp;
    }
}

Constructor looks like:

public Guesser()
{
    dataLength = dataCharset.Length;
    shuffle();
    Console.WriteLine(dataCharset);
}

And my main file where i am creating object
Guesser is my class

Guesser guesser1 = new Guesser();
Guesser guesser2 = new Guesser();
Guesser guesser3 = new Guesser();
Guesser guesser4 = new Guesser();

Results that i got

Why every object have the same array if i am creating it with 'new' keyword? I expected that each object will have its own shuffled array.

Please provide a complete code sample which compiles. Show this entire class, and show where you're creating multiple instances with the `new` operator, and what you're doing with them. The sample should be minimal, no more and no less than needed to demonstrate the issue; and it *should demonstrate the issue*, — 15ee8f99-57ff-4f92-890c-b56153, Jun 04 '19 at 19:49
Please update the **Question** with the code sample, not the comments — Rufus L, Jun 04 '19 at 19:52
Please add a complete, compilable code sample to your question. Please edit your question and add the code to your question. The sample should demonstrate the issue, and it should compile. It should include the entire, whole, complete, not partial, definition of the class you are creating, and it should show how you are creating it and what you're doing with it. It should compile. Not pastebin. Please add the code to your question. — 15ee8f99-57ff-4f92-890c-b56153, Jun 04 '19 at 19:53
1. You should be implementing a Fisher-Yates shuffle and you currently are not. 2. Your call to `new Random()` is not in the `for` loop, so I don't believe the previous answer applies. 3. (Essentially) the code you have above DOES result in different lists being displayed, so I suspect there is something in your code that you've left out above. — Clay Ver Valen, Jun 04 '19 at 20:30
@ClayVerValen What shuffling algorithm should i use then? Changing random to static outside of function worked. — Kulkejszyn, Jun 04 '19 at 21:04
@Kulkejszyn - Fisher-Yates, just like I wrote. In your current algorithm you are potentially reshuffling the item at position 0, even after it was shuffled in the first run of the for loop (and in position 1 in the second run, etc). A [search](https://www.google.com/search?q=fisher+yates+don+knuth&rlz=1C1SQJL_enUS784US784&oq=fisher+yates+don+knuth) for Fisher-Yates Don Knuth will show you what alterations you need to make. — Clay Ver Valen, Jun 04 '19 at 21:14

D Stanley · Accepted Answer · 2019-06-04T20:13:42.413

0

It's possible (though not as obvious as I usually see) that you're code is fast enough that you're using the same seed for your Random objects. Try making it a static property of the class:

static Random random = new Random();

void shuffle()
{
    for (int i = 0; i < dataLength; i++)
    {
        int index = random.Next(0, dataLength);
        char temp = dataCharset[index];
        dataCharset[index] = dataCharset[i];
        dataCharset[i] = temp;
    }
}

edited Jun 04 '19 at 20:13

answered Jun 04 '19 at 19:59

D Stanley

149,601
11
178
240

I updated my full array. I typed 1,2,3,4,5,6 to simplify it. So it is sill not correct? – Kulkejszyn Jun 04 '19 at 20:05
I tested OP's code with and without a Thread.Sleep() between the constructors. This answer is correct. – 15ee8f99-57ff-4f92-890c-b56153 Jun 04 '19 at 20:10
@Kulkejszyn Well, `char` is just a byte so it will _compile_ but they are not _printable_ characters. I was assuming that you had some way to see that the "shuffled" arrays were always the same and that the actual values were irrelevant. – D Stanley Jun 04 '19 at 20:13
@DStanley It won't compile unless there's an explicit cast on each int. In C#, a char has pretensions of being more than just a byte. Actually, it's two bytes. – 15ee8f99-57ff-4f92-890c-b56153 Jun 04 '19 at 20:19
@EdPlunkett Good point. In any case, I've removed that part from my answer since it's orthogonal to the problem. – D Stanley Jun 04 '19 at 20:20
@DStanley Yes, it was orthogonal, and it's not in the question any more anyhow. – 15ee8f99-57ff-4f92-890c-b56153 Jun 04 '19 at 20:21
You are still not shuffling properly. Use a [Fisher-Yates Shuffle](https://stackoverflow.com/questions/34194548/select-distinct-and-random-numbers-from-list/34196592#34196592). And if you feel your current shuffle is correct, please take it up with Don Knuth. – Clay Ver Valen Jun 04 '19 at 20:48

Each object have the same array

1 Answers1