Better way to select a random string from a List of strings

Question

I have a game of guessing words something like hangman.

This is the code I use to select a word from the words list:

List<string> words = GetWordsList();
int index = new Random().Next(words.Count);
string random = words[index];

Is there a better way to get a random item from the words list?

EDIT 1:

by better I mean (better performance or better randomness or other improvements to consider)

EDIT 2:

I call these lines every 15-30 seconds based on how much time it took to the player to guess the word.

EDIT 3:

I don't know if it is useful, but to have more information about the context, I remove the item from the list after these lines of code.

Solution

I would look at doing something like this:

var rnd = new Random();
var words = new Stack<string>(GetWordsList().OrderBy(w => rnd.Next()));

Then you just .Pop() the next value from the stack to get the next random word.

Just be mindful to instantiate the Random instance only once in your app to avoid producing non-random values related to calling your code in quick succession.

Comments

[Sriram Sakthivel]

How this is not better? IMO this is clean way of doing it.

[idstam]

If you want to do that a lot you can encaspulate it in a generic extension method.

[csharpwinphonexaml]

@all I am not sure if it is the best in performance and randomness. That's why I'm asking

[Marius Bancila]

You can keep the Random object and not create it with every call. If you call this function at very shorts intervals it might be that the time dependent value used for seeding the generator (when it's created) is the same. The result is that it produces the same numbers. So you should try to avoid that.

[Robert Langdon]

+1 for @Marius. Just an addition - with 15-30 seconds the seeding should be sufficiently distinct and random. You can continue with the solution stated above.

[Stefan Steinegger]

you should probably remove the word to avoid that it is used again. Alternatively, you sort it randomly once at the beginning.

[csharpwinphonexaml]

@StefanSteinegger I already do

[Enigmativity]

Then use a Stack<string> and .Pop() the values.

[Stefan Steinegger]

Does it work? Is the expression in the OrderBy clause only evaluated once per item? If not, it could have some strange side effects, for instance in performance.

[Enigmativity]

@StefanSteinegger - Yes, the OrderBy is only evaluated once per list element. Besides, even if it didn't the side effect would be randomizing the list anyway.