A VB convert...

[Jibrohni]

Hey all, I'm just starting out using C# and am trying to learn by converting a program I've started in VB to C#.

 

Sadly I'm stuck early on with trying to generate an array of 52 randomly ordered integers. I have it working but fear that I'm not doing a tidy job. The routine seems very sluggish as opposed to it running in VB. Please can anyone advise on a better way to do this than my method below please?

 

I'm using a goto statement which I have a bad feeling about and it's also doing a fair few iterations that can most likely be avoided. Even so, it shouldn't be taking my nice new i5 processor 3-4 seconds??

 

 

 

private int RandomNumber(int min, int max)
       {
           Random random = new Random();
           return random.Next(min, max);
       }
       public int[] GenerateDeck()
       {
           int[] DECK = new int[52];       //creates an array to hold thegenerated deck
           int iCounter = 1;

           do
           {
           Start:
               int rndNum = RandomNumber(0, 52);

               if (DECK[rndNum] == 0)
               {
                   DECK[rndNum] = iCounter;
                   iCounter++;
               }
               else
                   goto Start;
           } while (iCounter < 53);

           return DECK;

       }
private void btnGenerate_Click(object sender, EventArgs e)
       {

           GenerateDeck();

       }

I really appreciate your time. P.S. I like the efficiency and neatness of C# coding. Shame I'm crap at it...

[snarfblam]

I would start by using a for loop. These aren't too different between VB, the syntax is just painfully awkward and the terminating value is usually different by 1.

For [color="Blue"]I As Integer[/color] = [color="Green"]0[/color] To [color="Red"]9[/color]
Next

for([color="Blue"]int i[/color] = [color="Green"]0[/color]; i < [color="Red"]10[/color]; i++) { } // You could also change < 10 to <= 9

 

A for loop is the best way here. It makes it clear that you want to loop for 1 to 52.

 

Also, FYI, you can use the continue statement to go to the beginning of a loop. This could achieve the same effect as your goto, but we won't be needing this anyway.

 

So we could change your code to:

  int[] DECK = new int[52];     [color="Green"]  //creates an array to hold thegenerated deck[/color]
      [color="Red"] for(int iCounter = 1; iCounter < 53; iCounter++) {[/color]
           
               int rndNum = RandomNumber(0, 52);

               if (DECK[rndNum] == 0)
               {
                   DECK[rndNum] = iCounter;
                   iCounter++;
               }
               else
                    [color="Green"]// We'll be getting rid of this[/color]
                   [color="Red"]continue;
           } [/color]

           return DECK;

Now, one problem you have here is that you keep picking random numbers until you find an empty spot. Theoretically, this could potentially take infinitely long. Realistically, it's just gonna take a long time.

 

The method I prefer is to know how many empty spots you have, and pick a random empty spot. For instance, if we have ten empty spots, pick a random number between 0 and 9. Then, you walk through the array counting how many empty spots you pass until you find the one you want.

  int[] DECK = new int[52];       //creates an array to hold thegenerated deck
       [color="Green"]// Loop over the values we will insert into the deck[/color]
       for(int iCounter = 1; iCounter < 53; iCounter++) {

           [color="Green"]// Determine the # of empty spots and pick a random one[/color]
           int emptySpots = 53 - i;
           int insertIndex = RandomNumber(0, emptySpots);

           int currentLocation = 0; [color="Green"]// Index within DECK[/color]

           [color="Green"]// Count through empty spots until we find the one we want[/color]
           for(int emptySpotIndex = 0; emptySpotIndex <= insertIndex; emptySpotIndex++) {

               [color="Green"]// Find the next empty spot[/color]
               while(DECK[currentLocation] != 0) {[color="Green"] // Go until we find a zero (empty)[/color]
                   currentLocation++;
               }
           }

           [color="Green"]// currentLocation now points to the nth empty spot, which is where we want to add a value.[/color]
           DECK[currentLocation] = iCounter;
       }
       return DECK;

 

My computer is systematically failing one component at a time, and right now I'm limping along without C# installed, so I haven't tested this code.

[Jibrohni]

Thanks for your time on this I really appreciate it.

 

This will work much better than the way I was grinding out the result.

 

I feel your pain too. I've just built a new upgraded pc trying to save money, HA! It's so far cost me £900 and I'm just about to buy a graphics card. (And this didn't include a screen). Oh well, I'll have to make sure I get my moneys worth...