Problem with infinite loop when converting ASCII to Binary (in C)

Question

I'm writing a code to convert a text into ASCII and then to binary. The text->ASCII conversion works fine, but during the ASCII->binary conversion I get an infinite loop when I run the program. Which part of my code is wrong?

P. S. I'm working with the cs50 library provided by the Harvard CS50 course, hence the fourth line.

const int BITS_IN_BYTE = 8;
int main(void)
{
   string text = get_string("Text: ");
   int binary[BITS_IN_BYTE];
   for (int i=0, len = strlen(text); i<len; i++)
   {
        int ascii = text[i];
        printf("%d\n", ascii);

        for(i=0; ascii>0; i++)
        {
            ascii = ascii / 2;
            binary[i] = ascii % 2;
        }
        printf("Binary for the given number is: ");
        for(i=i-1; i>=0; i--)
        {
            printf("%d", binary[i]);
        }
    }
}

As well as reusing `i` when you shouldn't, I think the `ascii /= 2;` statement should come after, not before, the assignment to `binary[i]`. As it is, you throw the least significant bit away. (Oh, and you wrote the `/=` operator out longhand.) — Jonathan Leffler, Jul 30 '23 at 06:09

score 0 · Answer 1 · answered Jul 30 '23 at 05:39

The problem is that you're using i for both your outer and inner loops. At the end of the second inner loop, i is -1, which gets incremented by the outer loop to 0. So it just keeps processing the first element over and over. Use different variables for your inner loops.

score 0 · Answer 2 · answered Jul 30 '23 at 05:40

You are reusing the i variable several times.

You are using it to mean "the index into the string". You are using it to mean "the index into the ascii byte being read". You are using it to mean "the index into the ascii byte being written".

Give each of those usages a different, more meaningful, identifier.

paddy · Answer 3 · 2023-07-30T21:32:03.687

In the outer loop, you use i to count iterations. In both the inner loops, you overwrite i and re-use it for those loops. The second inner loop counts backwards past zero:

for(i=i-1; i>=0; i--)
{
    printf("%d", binary[i]);
}

So, after this loop, i is -1. The outer loop then increments that to zero for the next iteration, which is the same value as the previous iteration and so you have an infinite loop.

You should generally avoid reusing loop variables. Since the second inner loop relies on the count from the first, you could do it like this:

int bit = 0;
for(; ascii > 0 && bit < BITS_IN_BYTE; bit++)
{
    ascii = ascii / 2;
    binary[bit] = ascii % 2;
}
printf("Binary for the given number is: ");
for(bit = bit-1; bit >= 0; bit--)
{
    printf("%d", binary[bit]);
}

Note that it's not really necessary to have two loops and store things in an array, just to get this output. But maybe that's a course requirement or something.

Might want to grab the LSB BEFORE calculating and storing the individual bits. — Fe2O3, Jul 30 '23 at 05:50
After the second inner loop, `i` is `-1`, not `0`. Remember, the continuation condition is `i>=0`, not `i>0`. It's only after the outer loop increments it that it goes back to `0`. — Tom Karzes, Jul 30 '23 at 05:52

Problem with infinite loop when converting ASCII to Binary (in C)

3 Answers3