Getting random item from Dictionary? Not random

Question

I am running multiple threads on my application and in each thread I need to get a random Dictionary item from the account. Now I know of course once in a while you'll get the same item from a small dictionary, but this dictionary has over a thousand items and I am still getting non-unique results?

What do I mean by that? I mean it'll give random results but usually they repeat

Examples:

Picked the random username "aidan913"
Picked the random username "aidan913"
Picked the random username "abbiexox9"
Picked the random username "phelan193"
Picked the random username "pbeters92"

So It'll repeat it twice sometimes, but not actually give a fully unique item. Surely there has to be a way to get a unique item at least 9/10 times?

public KeyValuePair<int, BotInformation> GetAccount()
{
    var account = new KeyValuePair<int, BotInformation>();

    var rand = new Random();
    var randomlyOrdered = _accounts.OrderBy(i => rand.Next());

    lock (locker)
    {
        foreach (KeyValuePair<int, BotInformation> entry in randomlyOrdered)
        {
            if (!entry.Value.Usable())
                continue;

            account = entry;
        }
    }

    if (!CoreUtilities.IsDefault(account)) // if it found one, update it?
    {
        using (var databaseConnection = Program.GetServer().GetDatabaseManager().GetConnection())
        {
            databaseConnection.SetQuery("UPDATE `accounts` SET `last_used` = '" + 
                DateTime.Now.ToString("yyyy-MM-dd HH:mm:ss") + "' WHERE `username` = '" + account.Key + "' LIMIT 1");
        }

        account.Value.LastUsed = DateTime.Now;
    }

    return account;
}

you are picking the last `Usable` value from the randomized dictionary. Maybe there are only a few `Usable` among the thousands? — Cee McSharpface, Jul 16 '17 at 19:28
I think that you are also missing the point that random != unique. You best bet would be to keep a list of used items and ignore if it has been used — David Pilkington, Jul 16 '17 at 19:30
99% are avalible, isAvalible is basically checking if the last_used is more than an hour ago which they are. — Seriosk Official, Jul 16 '17 at 19:30
A sequence of random numbers will necessarily repeat itself. Just because you got heads the first time you flipped a coin, that doesn't mean you're guaranteed to get tails the next time. That said, your code has the obvious defect that you are creating a new `Random` object each time you want to select a random element, so if you try to select a random element too often, you'll definitely get the same element multiple times in a row. See marked duplicate for solution to that. — Peter Duniho, Jul 16 '17 at 19:39
See also https://stackoverflow.com/questions/108819/best-way-to-randomize-an-array-with-net for information on how to randomize your collection in a way that ensures against duplicate choices, i.e. by shuffling the data instead of picking a new random element each time. — Peter Duniho, Jul 16 '17 at 19:39

score 1 · Accepted Answer · answered Jul 16 '17 at 19:30

This is your problem

public KeyValuePair<int, BotInformation> GetAccount()
{
    var account = new KeyValuePair<int, BotInformation>();

    var rand = new Random();

When you new Random() quickly it will have the same key.
New Random() only once and the use it.

private var rand = new Random();
public KeyValuePair<int, BotInformation> GetAccount()
{
    var account = new KeyValuePair<int, BotInformation>();

score 0 · Answer 2 · answered Jul 16 '17 at 19:30

0

Take this line out of the function:

var rand = new Random();

And it would be better make the instance of the Random static. Read about Random class here

answered Jul 16 '17 at 19:30

Samvel Petrosov

7,580
2
22
46

Getting random item from Dictionary? Not random

2 Answers2