CODEWITHSUNDEEP
carrays
SivaDotRender
SivaDotRender

Reputation: 1651

Changing array values while iterating though the same array causes strange behaviour

I wrote a loop to replace every character in a string with another using a reference array. 

for(int i=0 ; i < encoded_message_len ; i++){

    for(int j=0; j < 26 ; j++){

        if(encoded_message_copy[i] == substitution_alphabet[j]){

            printf("%c ->>>> %c @ index:%d \n",encoded_message_copy[i], original_alphabet[j], i);
            encoded_message_copy[i] = original_alphabet[j];
        }
    }
}

When I run this code however, I get a strange output: 

J ->>>> C @ index:0
H ->>>> R @ index:1
Q ->>>> Y @ index:2
Y ->>>> Z @ index:2
S ->>>> P @ index:3
U ->>>> T @ index:4
T ->>>> V @ index:4
X ->>>> O @ index:6
F ->>>> L @ index:7
L ->>>> X @ index:7
X ->>>> O @ index:8
B ->>>> G @ index:9
G ->>>> W @ index:9
Q ->>>> Y @ index:10
Y ->>>> Z @ index:10

When I remove this line : encoded_message_copy[i] = original_alphabet[j]; from the loop, I get the expected output: 

J ->>>> C @ index:0
H ->>>> R @ index:1
Q ->>>> Y @ index:2
S ->>>> P @ index:3
U ->>>> T @ index:4
X ->>>> O @ index:6
F ->>>> L @ index:7
X ->>>> O @ index:8
B ->>>> G @ index:9
Q ->>>> Y @ index:10

Can someone explain why this is happening?

Upvotes: 0

Views: 60

Answers (5)

mctylr
mctylr

Reputation: 5169

/* msg_ptr is the position within the encoded message */
/* letter is a numeric value which corresponds to letter of the
   alphabet (0 maps to 'A', 1 => 'B', etc. */
for (int msg_ptr = 0 ; msg_ptr < message_len ; msg_ptr++) {
    for (int letter = 0; letter < 26 ; letter++) {
        if(encoded_message[msg_ptr] == substitution[letter]) {
           encoded_message[msg_ptr] = original[letter];
        }
    }
}

The problem may become clearer to you now. The problem is that while you modify encoded_message[msg_ptr] with the value from the original[letter], the inner loop (the letter for loop) is next to execute / increment. This means the if comparison is done again for an unchanged msg_ptr (i) and an incremented letter (j) values. This means the comparison is done with the freshly decoded value at encoded_message[msg_ptr].

Once you successfully find and replace the encoded message at a given position, you want to advance to the next character in the encoded_message array (i.e. increment the value of msg_ptr (i). This can be done using the break keyword.

Another site has some program execution diagrams for the break (and continue) statement, which may make it clearer.

If you have problems understanding why this is necessary (or in other words how nested loops operate), mentally run through the following example in your head, as well as your compiler to see.

for (int i = 0; i < 3; i++) {
    for (int j = 0; j < 4; j++) {
        printf("i=%d j=%d\n", i, j);
    }
}

The inner loop (j) by default will finish before the next iteration of the outer loop (i) occurs.

Upvotes: 1

Vlad from Moscow
Vlad from Moscow

Reputation: 310940

The problem is that when you use statement

        encoded_message_copy[i] = original_alphabet[j];

you do not break the loop. 

for(int j=0; j < 26 ; j++){

    if(encoded_message_copy[i] == substitution_alphabet[j]){

        printf("%c ->>>> %c @ index:%d \n",encoded_message_copy[i], original_alphabet[j], i);
        encoded_message_copy[i] = original_alphabet[j];
    }
}

So the loop continues to compare the new substituted character with other characters in substitution_alphabet.

You should break the loop if the character was already substituted

for(int j=0; j < 26 ; j++){

    if(encoded_message_copy[i] == substitution_alphabet[j]){

        printf("%c ->>>> %c @ index:%d \n",encoded_message_copy[i], original_alphabet[j], i);
        encoded_message_copy[i] = original_alphabet[j];
        break;
    }
}

Upvotes: 1

Andrew Henle
Andrew Henle

Reputation: 1

You're changing your data - an early iteration of the outer loop changes your data so it gets seen later.

Upvotes: 1

Andrey Derevyanko
Andrey Derevyanko

Reputation: 560

You're finding several symbols at the same position. You need to skip further looking at i-th symbol when you found it. Try to add break:

if(encoded_message_copy[i] == substitution_alphabet[j]) {
    printf("%c ->>>> %c @ index:%d \n",encoded_message_copy[i], original_alphabet[j], i);
    encoded_message_copy[i] = original_alphabet[j];
    break;
}

Upvotes: 1

KompjoeFriek
KompjoeFriek

Reputation: 3875

Put a break; after the line you needed to remove to make it correct (inside the for loops)

You change the contents of encoded_message_copy, this can cause the statement if(encoded_message_copy[i] == substitution_alphabet[j]) to match multiple times for the same value of i ;-)

Upvotes: 1

Related Questions