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

Question

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?

You do not break out of the inner loop when you find the substitution character. As a result, it is sometimes the case that the updated character is also found in substitution_alphabet, and substituted again. Put a break statement at the end of the body of your if statement. — John Bollinger
– John Bollinger, Commented Apr 22, 2015 at 20:20

KompjoeFriek · Accepted Answer · 2015-04-22 20:19:36Z

1

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

answered Apr 22, 2015 at 20:19

KompjoeFriek

3,9501 gold badge26 silver badges42 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Andrey Derevyanko · Accepted Answer · 2015-04-22 20:20:31Z

1

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

answered Apr 22, 2015 at 20:20

Andrey Derevyanko

5605 silver badges12 bronze badges

Comments

Andrew Henle · Accepted Answer · 2015-04-22 20:21:28Z

1

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

answered Apr 22, 2015 at 20:21

Andrew Henle

35k3 gold badges29 silver badges62 bronze badges

Comments

Vlad from Moscow · Accepted Answer · 2015-04-22 20:24:11Z

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

mctylr · Accepted Answer · 2015-04-22 20:58:00Z

/* 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.

Collectives™ on Stack Overflow

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

5 Answers 5

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related