Binary to Decimal in C. How can I improve it?

Question

I'm writing a C program that reads a pure binary number from the keyboard in the form of a string of characters (0 and 1) up to 24 bits long. the program must:

Check that the inserted string is correct, that is, it is composed only of 0 and 1
Convert bin to dec
Print the dec

#include <stdio.h>
#define MAXC 24

int main(void)
{
    char bin[MAXC + 1], dec[MAXC *2];

    int i, N, j, M;

    do {
        printf("Enter the binary number: ");
        gets(bin);
        N = strlen(bin);

        if (N > MAXC) {
            printf("Error: max 24 bit");
        }
    } while (N > MAXC);

    int corretto = 0;

    for (i = 0; i < N; i++) {
        if ((bin[i] != 0) || (bin[i] != 1)) {
            corretto = 1;
        }
        else {
            for (j = 0; j < MAXC * 2; j++) {
                dec[j] = bin[i] *pow(2, N - 1 - i);
            }
        }
    }

    M = strlen(dec);

    if (corretto == 1) {
        printf("Il numero binario non e' scritto correttamente");
    }
    else {
        for (j = 0; j < M; j++) {
            printf("Il numero decimale e': %c", dec[j]);
        }
    }

    return 0;
}

Hi Antonio. Can you be a little more precise in what exactly you are trying to improve and where you're hitting a wall? — R.A. Lucas, Feb 24 '21 at 12:07
`if(N>MAXC){` If that is ever true, you are already in the world of undefined behaviour because you array cannot hold any string longer than N chars. For that reason you should never use `gets`. See [here](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used) for more details. — Gerhardh, Feb 24 '21 at 12:09
Welcome to Stack Overflow. Please check out the [tour] and the [question guide](https://stackoverflow.com/help/how-to-ask) to enhance your question. You can [edit] your question with details at any time. — costaparas, Feb 24 '21 at 12:11
`dec[MAXC*2]` Why `*2`? You need approximately 1 decimal digit for 3 binary digits. Or for 24 bits you need maximum of 8 digits + `\0` — Gerhardh, Feb 24 '21 at 12:11
The `gets` function is unsafe. Use `fgets` instead, but note that `fgets` will leave the terminating newline character in the buffer if there is room for it. I suggest making the buffer at least 26 long (24 for the binary number, plus 1 for the newline, plus 1 for the null terminator). — Ian Abbott, Feb 24 '21 at 12:11
Please compile your code with a **recent** C standard. Enable many warnings and resolve all warnings. Turn all warnings into errors. Do not attempt to write or run code that uses the `gets` function. **Please, never use the [very dangerous and obsolete `gets` function](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used).** For `gcc`/`clang`, you could use options like `-Wall -Werror -Wextra -O2 -g -std=[...]` where `[...]` is some standard like `c11`. — costaparas, Feb 24 '21 at 12:13
`if((bin[i]!=0)||(bin[i]!=1))` doesn't do what you want. You want to accept the *characters* `'0'` and `'1'`. And then when you do `bin[i] * pow(2, N-1-i)`, you need to convert from a character value to a digit value. — Steve Summit, Feb 24 '21 at 13:09
You don't want `dec` to be another array of characters, you want it to be a single `int`. (And you want to print it out at the end using `%d`, not `%c`.) — Steve Summit, Feb 24 '21 at 13:11
Please understand `if((bin[i]!=0)||(bin[i]!=1))` is functionally identical to `if (true)`. — Ruud Helderman, Feb 24 '21 at 13:12

score 0 · Answer 1 · answered Feb 24 '21 at 12:19

In terms of improvement you could do the following.

For each bit in the binary number you are looping over all digits over the decimal number this could be done in a rather simpler and more efficient way. Assuming dec is an int variable which stores the result you could do something like running a for loop over the bits and each iteration computes the contribution of that bit to the answer as in dec = dec + (bin[i] - '0') * current_power, current_power is initalized to 1 and is multiplied by 2 every iteration. This saves you from repeatedly calling pow function.
Finish the correctness check of the number before you start off with any computation. As in case if the number is invalid you don't need to do any computations.

masterop · Answer 2 · 2021-02-24T13:10:28.220

There are many optimizations that you could do here other then avoid using unsafe gets function:

Your check for '0' or '1' is wrong. Either it should be bin[i] == '0' or bin[i] - 0x30 == 0
Why the array for holding decimal value is double the size of binary?
Instead of using pow function use << operator
If you just have to print decimal then no need to save decimal numbers individually in an array, an int variable should be better and in that case you can save the for loop

score 0 · Answer 3 · answered Feb 24 '21 at 13:21

there are some bugs. I fixed them in the following code.

#include <stdio.h>
#include <string.h>
#include <math.h>

#define MAXC 24

int main(void)
{

        char bin[MAXC+1], dec[MAXC*2];

        int i, N, j, M;

        do{
                printf("Inserire il numero bin: ");
                fgets(bin, MAXC+1, stdin);
                N=strlen(bin);
                if(bin[N-1]=='\n'){       // remove '\n'
                        bin[--N] = '\0';  //
                }                         //

                if(N>MAXC){
                        printf("Error: max 24 bit");
                }
        }while(N>MAXC);

        int corretto=0;
        int dec_num = 0;

        for(i=0; i<N; i++){
        //      if((bin[i]!=0)||(bin[i]!=1)){ // it's still char. and & should be used than ||, or logic is always 1
                if((bin[i]!='0')&&(bin[i]!='1')){ // it's still char
                        corretto=1;
                        break;
                }
                if(bin[i]=='1'){
                        dec_num +=  pow(2, N-i-1);  // just used a decimal number, string is no longer nessisary
                }
        }


        if(corretto==1){
                printf("Il numero binario non e' scritto correttamente\n");
        }else{
                printf("Il numero decimale e': %d\n", dec_num);
        }

        return 0;
}

Binary to Decimal in C. How can I improve it?

3 Answers3