Why can't I swap out the variable of the for loop with a random letter?

Question

I was writing an integer to roman numeral converter. This time, a proper one.

Trying to solve the problem of printing the 900s, 90s and 9s; I wrote a for loop (lasts only one loop since it's purpose was to prevent printing CMCMCMCM instead of CM) inside the while loop. But for the variable of the for loop, I used the same "n" as the "n" in the while loop without noticing.

Ran it, worked fine; then noticed the supposed error. Swapped it out with "A", inserted "A" into int cd, cl,..... list. AAAaaand the result was the jumble of completely off the rail numbers.

I don't understand how the code can work properly with a reused loop variable, but can't work with a new one.

#include<stdio.h>
int main()
{
    int num, m, c, d, l, x, v, i, n, cm, cd;
    printf("Enter Number: ");
    scanf_s("%d", &num);
        m = num / 1000;
        d = (num % 1000) / 500;
        c = (num % 500) / 100;
        l = (num % 100) / 50;
        x = (num % 50) / 10;
        v = (num % 10) / 5;
        i = (num % 5);
        n = m + d + c + l + x + v + i;
        while (n > 0)
        {
            {
                for (m; m > 0; m--)
                {
                    printf("M");
                }
            }
            {
                for (n=1; n>0; n--)
                {
                    if (num%1000>=900)
                    {
                        printf("CM");
                        d = d - 1;
                        c = c - 4;
                        num = num - 900;
                    }
                }
            }
            {
                for (d; d > 0; d--)
                    printf("D");
            }
            {
                for (n = 1; n > 0; n--)
                {
                    if (num % 500 >= 400)
                    {
                        printf("CD");
                        c = c - 4;
                    }
                }

            }
            {
                for (c; c > 0; c--)
                    printf("C");
            }
            {
                for (l; l > 0; l--)
                    printf("L");
            }
            {
                for (x; x > 0; x--)
                    printf("X");
            }
            {
                for (v; v > 0; v--)
                    printf("V");
            }
            {
                for (i; i > 0; i--)
                    printf("I");
            }
            n--;
        }
        return 0;
}

Keep in mind, the for loops for the 900, 400 and so on are not completed as I noticed this after writing the 900 single loop.

Answer 1

First, you calculate the quantity of each different letter in the output. Then you have a while loop running with n decreasing to 0, i.e. the body of the loop runs n times. Inside the loop, you have a series of for loops, one for each different letter. If you leave out the for loops that use n, you're doing this:

• Calculate the number of each letter in the output: m, d, c, etc. as well as n which is the total number of letters (in additive notation).
• Repeat n times:
  • Print M, m times.
  • Print D, d times.
  • etc.

The result is that without the inner loops that modify n, you're printing the result many times over.

A correct algorithm for printing roman numerals in additive notation (4 is IIII, etc.) is:

• Calculate m, d, c, etc.
• Print M, m times.
• Print D, d times.
• etc.

Without the for loops that modify n, you're repeating the correct algorithm n times:

while (n > 0) {
    … /* code that doesn't modify n and prints the desired output */
    n--;
}

With the for loops that modify n, you're setting n to 0 inside the body of the while loop, so n-- at the end of the loop body sets n to -1, and the while loop terminates after the first iteration.

while (n > 0) {
    … /* some code */
    for (n = 1; n > 0; n--) {
        … /* more core */
    }
    /* On exit of this for loop, the value of n is 0. */
    … /* more code that doesn't modify n */
    n--;
}

The fix is simple: remove the while loop, since you always want to execute its body exactly once. Remove the calculation of the total number of letters to print: it isn't needed. Your for loops using n execute their body exactly once, too, so they're useless as well. This is the result (I've made the indentation uniform and removed useless braces, but otherwise I didn't seek to improve the program):

#include<stdio.h>
int main()
{
    int num, m, c, d, l, x, v, i, cm, cd;
    printf("Enter Number: ");
    scanf("%d", &num);
    m = num / 1000;
    d = (num % 1000) / 500;
    c = (num % 500) / 100;
    l = (num % 100) / 50;
    x = (num % 50) / 10;
    v = (num % 10) / 5;
    i = (num % 5);
    for (m; m > 0; m--)
    {
        printf("M");
    }
    if (num%1000>=900)
    {
        printf("CM");
        d = d - 1;
        c = c - 4;
        num = num - 900;
    }
    for (d; d > 0; d--)
        printf("D");
    if (num % 500 >= 400)
    {
        printf("CD");
        c = c - 4;
    }
    for (c; c > 0; c--)
        printf("C");
    for (l; l > 0; l--)
        printf("L");
    for (x; x > 0; x--)
        printf("X");
    for (v; v > 0; v--)
        printf("V");
    for (i; i > 0; i--)
        printf("I");
    return 0;
}