c++dynamicshuffle

Function behaves badly when passing dynamically allocated pointer


I have this function

void shuffle_array(int* array, const int size){
  /* given an array of size size, this is going to randomly
   * attribute a number from 0 to size-1 to each of the
   * array's elements; the numbers don't repeat */
  int i, j, r;
  bool in_list;
  for(i = 0; i < size; i++){
    in_list = 0;
    r = mt_lrand() % size; // my RNG function
    for(j = 0; j < size; j++)
      if(array[j] == r){
    in_list = 1;
    break;
      }
    if(!in_list)
      array[i] = r;
    else
      i--;
  }
}

When I call this function from

int array[FIXED_SIZE];
shuffle_array(array, FIXED_SIZE);

everything goes all right and I can check the shuffling was according to expected, in a reasonable amount of time -- after all, it's not that big of an array (< 1000 elements).

However, when I call the function from

int *array = new int[dynamic_size];
shuffle_array(array, dynamic_size);
[...]
delete array;

the function loops forever for no apparent reason. I have checked it with debugging tools, and I can't say tell where the failure would be (in part due to my algorithm's reliance on random numbers).

The thing is, it doesn't work... I have tried passing the array as int*& array, I have tried using std::vector<int>&, I have tried to use random_shuffle (but the result for the big project didn't please me).

Why does this behavior happen, and what can I do to solve it?


Solution

  • Your issue is that array is uninitialized in your first example. If you are using Visual Studio debug mode, Each entry in array will be set to all 0xCC (for "created"). This is masking your actual problem (see below).

    When you use new int[dynamic_size] the array is initialized to zeros. This then causes your actual bug.

    Your actual bug is that you are trying to add a new item only when your array doesn't already contain that item and you are looking through the entire array each time, however if your last element of your array is a valid value already (like 0), your loop will never terminate as it always finds 0 in the array and has already used up all of the other numbers.

    To fix this, change your algorithm to only look at the values that you have put in to the array (i.e. up to i).

    Change

    for(j = 0; j < size; j++)
    

    to

    for(j = 0; j < i; j++)