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);
}

}
``````
4
Contributors
31
Replies
119
Views
4 Years
Discussion Span
Last Post by 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.
}
``````

Edited by Ancient Dragon

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.

Edited by kingkong123

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

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';
}
``````

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);
}
``````

Edited by Ancient Dragon

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.

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];`

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?

Edited by kingkong123

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

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

Edited by kingkong123

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'

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] << " ";
}
}
``````

You could write your display like this

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

Edited by NathanOliver

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;
}
``````

Edited by Ancient Dragon

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] << " ";
}
``````

Edited by kingkong123

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;
}
}
``````

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

Edited by kingkong123

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.

Edited by Ancient Dragon

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] << " ";
}

}
``````

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.

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

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.

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

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

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

Edited by kingkong123

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.

Edited by Ancient Dragon

While or for?
Something like this:

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

Edited by kingkong123

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
}
``````

Edited by Ancient Dragon

It worked. Thanks for all your help.

This question has already been answered. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.