My random number generator does not create a new value in my loop C#

Question

I am using c# to try and populate a datagridview with random numbers but for some reason I keep getting the same value in all the cells.

    public int GenerateRandomNumber()
    {
        const int minimumNumber = -9;
        const int maximumNumber = 15;
        var random = new Random();
        var randomNumber = random.Next(minimumNumber, maximumNumber);
        return randomNumber;
    }

gameBoard is the datagrid view.

    private void populateButton_Click(object sender, EventArgs e)
    {
        CreateGameBoard((int)columnsNumericUpDown.Value,(int)rowsNumericUpDown.Value);
        gameBoard.RowTemplate.Height = gameBoard.Height/gameBoard.RowCount;
        foreach (DataGridViewRow row in gameBoard.Rows)
        {
            foreach (DataGridViewCell cell in row.Cells)
            {

                cell.Value = GenerateRandomNumber();
            }
        }
    }

See also: http://csharpindepth.com/Articles/Chapter12/Random.aspx — Jon Skeet, Sep 02 '13 at 06:19

Dmitry Bychenko · Answer 1 · 2013-09-02T06:23:29.443

5

Do not re-create random generator:

// Random generator, one instance for the loop, no re-creation 
private m_Random = new Random();
...
public int GenerateRandomNumber()
{
    const int minimumNumber = -9;
    const int maximumNumber = 15;

    // Do not re-create random generator! Use single instance instead
    //var random = new Random();
    var randomNumber = m_Random.Next(minimumNumber, maximumNumber);
    return randomNumber;
}

edited Sep 02 '13 at 06:23

answered Sep 02 '13 at 06:09

Dmitry Bychenko

180,369
20
160
215

4

This will not work properly in a multi-threaded environment, as `Random` is not thread-safe. You should *at least* mention this in the answer, even if you don't provide a thread-safe alternative. – Jon Skeet Sep 02 '13 at 06:19

score 4 · Accepted Answer · edited May 23 '17 at 12:22

Random is initialized using the clock which means that you will get the same value multiple times.

So, instead of re-creating Random instance you should keep a single Random instance and keep using Next on the same instance. If you have multiple threads then you should read this for locking solution

This is what you need.

private Random random = new Random();
const int minimumNumber = -9;
const int maximumNumber = 15;
public int GenerateRandomNumber()
{        
    var randomNumber = random.Next(minimumNumber, maximumNumber);
    return randomNumber;
}

My random number generator does not create a new value in my loop C#

2 Answers2