Shuffling an 2d array not executing properly?

[kingkong123]

Hi, there. I have a 2D array with 208. I must concatenate the strings by twos up to index 199.
So that I have 108 strings. Then I must shuffling these 108 strings.
The problem is that it isn't working right. After I concatenate the strings, then shuffle, several strings un concatenate again.
Ex:
born dramatic insurance gravy release frying
costume despair mission gibberish backyard abusive

I want to only concatenate the first 10 strings.
Result:
borndramatic insurancegravy releasefrying
costumedespair missiongibberish backyard abusive

Then shuffle:
abusive releasefrying backyard costumedespair
borndramatic missiongibberish insurancegravy

This is what my functions do:
abusive dramatic releasefrying gravy backyard costumedespair
borndramatic missiongibberish despair insurancegravy

It copies the second string again. I think the problem is in the concatenation function. How can I fix it

void Concatenation( char dest[208][13] )
  {
    for(int i=0; i<208 && i<200; i+=2)  // i < 208 concatenates all 208. 
                             // if I change to i < 200.  
     {
       myStrCat(dest[i],dest[i+1]); // myStrCat is equal to strcat().
       // cout << dest[i] << " "; // outputs the first 100.
     }


    for(int i=200; i<208; i++) 
     {
       // cout << dest[i] << " "; // outputs the last 8.
     }


     void ShuffleArray( char destination[208][13] )
{
   char temp[13];

   for (int i = 0 ; i < 208 ; i++)
     { 
       int m = rand() % 108;   
       myStrCpy(temp,destination[m]);   // myStrCpy is strcpy()
       myStrCpy(destination[m],destination[i]);
       myStrCpy(destination[i],temp);
     }


}

[Ancient Dragon]

The problem is in concantination function. The loop doesn't work right. After the first iteration the strings are

borndramatic dramatic insurance gravy release frying

Notice that dramatic is now in the list twice. When the first loop is finished the value of i is incremented from 0 to 2, which points to insurance. Now after the second loop iteration the strings are

borndramatic dramatic insurancegravy gravy release frying

This behavior is why after the shuffle some of the original strings are still there. I think what you need is another counter that tells where in the array the new strings should appear. In the code below myStrCat() had three parameters, not one. The first parameter is the destination, and the other two are the two strings to be concantinated. After the loop is finished you will have to null out all remaining strings in the array.

 for(int i=0, j = 0; i<208; i+=2, ++j)  // i < 208 concatenates all 208. 

     {
       myStrCat(dest[j], dest[i],dest[i+1]); // myStrCat is equal to strcat().
       // cout << dest[i] << " "; // outputs the first 100.
     }

[kingkong123]

I understand, but myStrCat has too many arguments. I only have two arguments. Would I have to change it? I have the code and can show you.

[Ancient Dragon]

yes, you didn't post it so I can't tell you how to change it.

[kingkong123]

Here it is:

void myStrCat( char dest[], char src[] )
{
   // Initialize variables
   int srcIndex = 0;
   int destIndex = myStrLen( dest );

   // loop through
   for (  ;src[srcIndex] != '\0'; srcIndex++, destIndex++  )
     {
      dest[destIndex] = src[srcIndex];
     }
   dest[destIndex] = '\0';
  }

[Ancient Dragon]

These my not work if dest is the same things as src1 (such as when i and j are both the same value in the loop). If that is the cast then you would only want to concantinate src2 to dest and ignore src1.

void myStrCat(char dest[], char src1[], char src2[] )
{
    if( dest != src1 ) // compare the pointers, not the strings
       strcpy(dest,src1);
    strcat(dest,src2);
}

[kingkong123]

I looks good but when I run the program, it freeze in the middle of the function and windows prompts me to close the program.(.exe has stopped working) Behind the prompt, I can see that it works.

[Ancient Dragon]

The main problem is that dest isn't big enough, some strings are longer than 13 characters. Increase the size of dest

char destination[208][25];

[kingkong123]

That fix it! Now when I output it and look closely, 8 strings are un concatenated. They aren't the ones that should be. How would I modifly the statements inside the for() so that it leave the last 8 string of the original alone?

[kingkong123]

Now there's 16 strings un concatenated. The final 8 and 8 other that should be.

[kingkong123]

Never mind the prevoius posts. i want to display the final product in another function using this:

for( i = 0; i < 108; i++)
    {
      cout << destination[i] << " ";
    }

The problem is that it doesn't output the last unconcatenated string. Instead it dispalys 8 other unconcatenated

Should be this:
abusive releasefrying backyard costumedespair
borndramatic missiongibberish insurancegravy

Instead i'm getting this:
repair releasefrying gravy costumedespair
borndramatic missiongibberish insurancegravy

[Ancient Dragon]

Your loop to display the output is wrong. After concantination there are no longer 108 strings. One way to solve that is after concantination zero out the first byte of all the remaining strings, then when displaying them don't use 108 but check for destination[i][0] == '\0'

[kingkong123]

Like this:

void Concatenation( char dest[MAX_S_ROWS][MAX_S_COLS] )
  {
    for(int i=0, j = 0; i<208 && i<200; i+=2, ++j)  

     {
       myStrCat2(dest[j], dest[i],dest[i+1]); 
       dest[i][0] = 0.0;

       // cout << dest[i] << " ";
     }

 }

 // Display

 for( i = 0; i < 108; i++)
    {
     if(destination[i][0] != '\0' ){
     cout << destination[i] << " ";
        }
    }

[NathanOliver]

You could write your display like this

for (int i = 0; destination[i][0] != '\0'; i++)
    cout << destination[i] << " ";

[Ancient Dragon]

for(int i=0, j = 0; i<208 && i<200; i+=2, ++j)

What's up with the i<208 && i<200? You only need one of those, not both. i<200 will prevent the loop from executing more than 200 times, so i<208 is useless code.

dest[i][0] = 0.0;

Careful with that because when i and j are the same value it will null out the string that it just finished concantinating. It would be a lot safer to do that in another loop

while( j < MAX_S_ROWS )
{
    destination[j++][0] = 0;
}

[kingkong123]

Ok. I'm now getting .exe has stopped working. This is how the functions look.
Concatenate

oid Concatenation( char dest[MAX_S_ROWS][MAX_S_COLS] ) // max_s_row = 208
  {                                                    // max_s_cols = 25;
   for(int i=0, j = 0; i<200; i+=2, ++j)  
     {
      myStrCat2(dest[j], dest[i],dest[i+1]); 

       while( j < MAX_S_ROWS ) // this loop is making the program close
        {    
         dest[j++][0] = 0;
        }
     }
 }

void display(  char destination[MAX_S_ROWS][MAX_S_COLS] )
{
   for (int i = 0; destination[i][0] != '\0'; i++)
    cout << destination[i] << " ";

}



void ShuffleArray( char destination[MAX_S_ROWS][MAX_S_COLS] )
{
  char temp[13];
  for (int i = 0 ; i < 108; i++)
   { 
    int m = rand() % 108; 
    myStrCpy(temp,destination[m]);
    myStrCpy(destination[m],destination[i]);
    myStrCpy(destination[i],temp);

    //cout << destination[i] << " ";
   }

[NathanOliver]

you need to move the while loop outside of your for loop

void Concatenation( char dest[MAX_S_ROWS][MAX_S_COLS] ) // max_s_row = 208
{                                                       // max_s_cols = 25;
    for(int i=0, j = 0; i<200; i+=2, ++j)
    {
        myStrCat2(dest[j], dest[i],dest[i+1]);
    }
    while( j < MAX_S_ROWS ) // this loop is making the program close
    {
        dest[j++][0] = 0;
    }
}

[kingkong123]

int j = 0 before the loops? and it seems like this new loop breaks the display function.

[Ancient Dragon]

you need to declare j counter above and outside both loops. I don't see how it could break the display function.

The loop in ShuffleArray() need to be fixed too -- when that function starts there it not 208 elements in the array. Use the same loop that's in display function.

[kingkong123]

I really appreciate your help guys!
That fix it. Now it doesn't display everything. It displays only several of the strings and sometimes more or less.
Heres all the function:

void Concatenation( char dest[MAX_S_ROWS][MAX_S_COLS] ) // max_s_row = 208
{            
    int i, j;                                       // max_s_cols = 25;
    for( i=0, j = 0; i<200; i+=2, ++j)
    {
        myStrCat2(dest[j], dest[i],dest[i+1]);
    }
    while( j < MAX_S_ROWS ) // this loop is making the program close
    {
        dest[j++][0] = 0;
    }


}

void ShuffleArray( char destination[MAX_S_ROWS][MAX_S_COLS] )
{
    char temp[13];

  for (int i = 0; destination[i][0] != '\0'; i++)
    { 
    int m = rand() % 108; 
    myStrCpy(temp,destination[m]);
    myStrCpy(destination[m],destination[i]);
    myStrCpy(destination[i],temp);

    cout << destination[i] << " ";
   }


}


void display(  char dest[MAX_S_ROWS][MAX_S_COLS] )
{
   // Initialize variables
   int i;

   // Loop and save the array data to file
   for (i = 0; dest[i][0] != '\0'; i++)
    {
      cout << dest[i] << " ";
    }


}

[Ancient Dragon]

line 22: there are not 108 elements in the array. You need to count the number of elements so that you can plug that number into the formula on line 22.

[kingkong123]

I counted 100 and it displays 100 now. Do you know were those last 8 strings are? I should have 108.

[Ancient Dragon]

Have you studies linked lists yet? This would be a lot easier to do if you could put the concantinated strings into a linked list. I have no idea where the other strings are, if you started out with 208 you should wind up with 104.

[kingkong123]

No I haven't. I start with 208 strings. Concatatenate 200 of it.
End with 108. Shuffle the 108.

[Ancient Dragon]

If you contantinate 200 strings how do you wind up with 108 strings? The math doesn't add up.

[kingkong123]

Because of the 8 unconcatenated one. The last 8 strings out of the 208 aren't concatenated 208 > 200 > 100 + 8

[Ancient Dragon]

The remaining 8 are still where they always were -- at the end of all the strings you null'ed out which makes them unaccessable. In the concantination function you will need to move them just after the last concantinated string. You will have to put a 3d loop in that function between the two loops you already have.

[kingkong123]

While or for?
Something like this:

for( j = 100; j < 107; j++ )
   {
    dest[j]; // no clue what goes here
   }

[Ancient Dragon]

In the first loop the j counter contains the value of the last concantinated string, so you want to copy the strings from i to j

for(; i < MAX_S_ROWS; j++, i++ )
   {
      strcpy(dest[j], dest[i]); // no clue what goes here
   }

[kingkong123]

It worked. Thanks for all your help.