Two random dice

[JustLetMeRegister]

Hello,

I'm trying to make a program that notably allows the user to roll two six-sided dice at the same time, and display the result on screen.

For some reason, it doesn't work properly, as the result of each dice is always the same as the other one (it'll always be 1 and 1, or 2 and 2, or 3 and 3, etc.). I don't know why. In the same program, I also allow the user to roll single dice separately, and that, on the other hand, works: it gets different results. It works in the exact same way, except it only rolls one dice at a time instead of two.

I use public functions. Here is the code that calls them when the user clicks on a button:

RollDice1()
        Select Case RollDice1()
(...) ' here I insert code that changes an image to display the dice result on screen
        End Select
RollDice2()
        Select Case RollDice2()
(...) ' ditto
        End Select

And here are the functions:

    Public Function RollDice1() As Byte
        Dim R1 As New Random()
        Dim R2 As Byte
        R2 = R1.Next(1, 7)
        RollDice1 = R2
    End Function
    Public Function RollDice2() As Byte
        Dim R3 As New Random()
        Dim R4 As Byte
        R4 = R3.Next(1, 7)
        RollDice2 = R4
    End Function

I know both functions are exactly the same but I figured maybe creating a second one, with different variables, would help to get two different results... but no. Both results are always the same, for a reason that escapes me.

Any help would be GREATLY appreciated!

PS: I am new to Visual Basic so maybe it's a dumb mistake... I wouldn't know.

 

[jimmajsterski]

try using the Randomize Statement to reseed the Random number generator.

 

[jmcilhinney]

try using the Randomize Statement to reseed the Random number generator.

Definitely not. The Randomize statement is for use with the Rnd function. They are holdovers from VB6 and noone should be using either in VB.NET.

First of all, I have added code formatting tags to your snippets. Please do so for us in future because, as you can see, it is much easier to read.

As for the code, I can see several issues. Firstly, you should not be creating a Random object inside the methods. When you create a Random object without specifying a seed, it uses the system time as a seed. If you create multiple Random objects in quick succession they will all use the same seed and therefore all generate the same pseudo-random sequence of numbers. As such, calling Next once on each will produce the same number multiple times. What you should be doing is creating one and only one Random object and calling Next on it multiple times. As such the Random object should created outside all the methods, using a member variable.

Secondly, while it's not illegal, it's also a holdover from VB6 to assign a value to the function name and let it be returned implicitly. The proper way is to use an explicit Return statement in all functions. You can have multiple Return statements if you wish, e.g. in an If block and an Else block but I subscribe to the school of thought that says that you should only ever have one Return statement at the end of a method.

Finally, while it depends on what you actually have in the Cases, most likely you should not be using a Select Case either. If all you're doing is selecting an Image then you should place all the Images in an array and then use your random number as an index into that array to retrieve the appropriate Image with a single line of code.

Out of interest if nothing else, you may like to check out this thread of mine on rolling dice:

Rolling Dice

 

[JustLetMeRegister]

Thanks for all the advice, I'll try that.

 

[jimmajsterski]

The thread "Rolling Dice" uses IRandomNumberGenerator and GenerateRandomNumber. I will use these from now on.
But I wonder why Rnd and Randomize are still in 2010 help and in "Visual Basic 2010 Step by Step" by Michael Halvorson ?
Still usable but not preferred ?

 

[jmcilhinney]

But I wonder why Rnd and Randomize are still in 2010 help and in "Visual Basic 2010 Step by Step" by Michael Halvorson ?
Still usable but not preferred ?

There are a number of modules defined in the Microsoft.VisualBasic namespace that contain a VB.NET implementation of all the functions that were built into VB6. That was basically done so that code upgraded from old VB6 project would work with as few changes as possible. Randomize and Rnd are declared in the VBMath module. Other such functions include MsgBox, InputBox, etc. Unfortunately, many developers who have migrated from VB6 continue to use those same functions that they used to use rather than using the standard .NET alternatives. Even more unfortunately, some of those developers write books and reach classes, thus propagating these undesirable practices to VB.NET beginners with no VB6 baggage, who should be starting VB.NET with a clean slate and doing things the right way.