CODEWITHSUNDEEP
carraysrandom
maxisme
maxisme

Reputation: 4245

Randomising results in array

I am trying to randomise an array and I have succesfully done it. But it is taking over 10 seconds to put out 16 (PLAYERS) numbers and it is using 99% cpu!!

Why is that?

Here is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#define PLAYERS 16

void getNames(char names[PLAYERS][NAME_LEN])
{
int x, y[PLAYERS] = {16,16,16,16,16,16,16,16,16,16,16,16,16,16,16,16},
    z, a = 1,
    b = 0;
char tempNames[PLAYERS][NAME_LEN];

while (a == 1)
{
    srand ( time(NULL) );
    int random_number = rand() % PLAYERS;
    while (1)
    {
        if (y[b] == random_number)
        {
            break;
        }
        else
        {
            b++;
            strcpy(tempNames[b - 1], names[random_number]);
            y[b] = random_number;
            if (b == PLAYERS)
            {
                a = 0;
                
                /*Move temp array back to the original array*/
                for (z=0; z<PLAYERS; z++) {
                    strcpy(names[z], tempNames[z]);
                }
            }
        }
    }
}
}

EDIT

I have just noticed that this code doesn't pick an original value for each random number!

Upvotes: 0

Views: 89

Answers (3)

Scott Hunter
Scott Hunter

Reputation: 49803

Part of the problem is that you keep seeding the random number generator; move the call to srand outside the loop.

This probably isn't slowing things much, but your inner loop shouldn't be a loop; either it breaks, or it sets the condition to cause a break the next iteration.

Here's something much simpler:

srand ( time(NULL) );
char tempname[NAME_LEN];
for ( a = PLAYERS-1; a>0; a-- ) {
    // Pick a position to swap with last name
    int r = rand() % (a+1);
    if ( r == a )
        continue;
    // Swap names[a] and names[r]
    strcpy( tempname, names[r] );
    strcpy( names[r], names[a] );
    strcpy( names[a], tempname );
}

Upvotes: 1

Weather Vane
Weather Vane

Reputation: 34585

You have incremented b and then used it before checking its range. So when b == PLAYERS you have already corrupted something else.

b++;
...
y[b] = random_number;
if (b == PLAYERS)

Upvotes: 0

Philip
Philip

Reputation: 1551

First off, things that bug me to no end that don't particularly pertain to your question.

  • The system you have of having a forever loop with a break condition is bad mojo. That condition should be in the while loop condition.
  • Why does that first loop exist? while(a==1)? is there a chance that incrementing b will never equal PLAYERS? It really really shouldn't. And that loop and flag can be axed.

You're getting a random number, quitting if the array at the NEXT index equals this random number, setting the array point to the number, and then doing stuff.

I believe you're exiting that inner forever loop due to luck.

Debug it, and break when b == PLAYERS. Look at what happens at the next iteration. Does it exit the loop when you want it to?

Upvotes: 0

Related Questions