HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcMinor

Convert integers into binary

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
convertintointegersbinary

Problem

I've made a very simple program that takes integer number arguments either with plus/minus sign or without them, and print the number in binary format.

binary.c

#include 

void printBinary(int);
int toInt(int*, char[]);
int handleProgram(char[]);

int main(int argc, char *argv[])
{   
    return handleProgram(argv[1]);
}

int handleProgram(char param[])
{
    int number, state;

    if( toInt( &number, param ) != -1 )
    {
        printBinary(number);

        state = 0;
    }
    else
    {
        printf("\tmain()::Exiting program...\n");

        state = -1;
    }

    return state;
}

void printBinary(int number)
{
    for (int i = 31; i >= 0; i--)
    {
        if (number >> i & 1 == 1)
            putchar('1');
        else
            putchar('0');

        if (i != 0)
        {
            if (i % 4 == 0)
                putchar(' ');

            if (i % 16 == 0)
                printf(": ");
        }
    }

    printf("\n");
}

int toInt(int *num, char param[])
{
    *num = 0;
    int sign = 1;
    int i = 0;

    if(param == NULL)
    {
        printf("\ttoInt()::No parameter, please enter a valid number!\n");

        return -1;
    }

    if(param[i] == '-')
    {
        sign = -1;
        i++;
    }
    else if( param[i] == '+' )
    {
        i++;
    }

    // check that a number exist
    if(param[i] == '\0')
    {
        printf("\ttoInt()::No number detected, please enter a valid number\n");

        return -1;
    }

    for(; param[i] != '\0'; i++)
    {
        if(param[i] >= '0' && param[i] <= '9')
            *num = 10* *num + param[i] - '0';
        else
        {
            printf("\ttoInt()::Invalid parameter, please enter a valid number!\n");

            return -1;
        }
    }

    *num *= sign;

    return 0;
}


Sample output: positive and negative cases

`D:\C\chapter2>gcc binary.c -o binary.exe

D:\C\chapter2>binary 3
0000 0000 0000 0000 : 0000 0000 0000 0011

D:\C\chapter2>binary +3
0000 0000 0000 0000 : 0000 0000 0

Solution

Functionality

-
Why 31? Why not 15 or 63 or 42? If code is to handle 32-bit integers, then use long as that type is at least 32-bit, rather than int, which can be as small as 16. Alternatives int32_t, int_least32_t. Even better: re-code for the largest integer type intmax_t.

// Why 31?
for (int i = 31; i >= 0; i--)


-
Right shifting a negative number leads to implementation defined behavior. Consider unsigned instead.

// ... (int number)
... (unsigned number)
... 
if (number >> i & 1 == 1)


-
Good that code allows a leading '+'.

-
Pedantic point. int overflow is undefined behavior and that is the result of the following with argv[1] == "-2147483648", a string that should "work".

*num = 10* *num + param[i] - '0';


-
Avoid using printf(some_string). Should some_string contain a %, maybe due to a code update, that will invoke UB as printf() expects the first arg to be a format string.

// printf(": ");
fputs(": ", stdout);
// or (2nd choice)
printf("%s", ": ");


-
Do not use argv[1] unless it is valid

int main(int argc, char *argv[]) {   
  if (argc > 1) return handleProgram(argv[1]);
  else ...


-
Prefer terminating error messages to go to stderr

// printf("\ttoInt()::No number detected, ...
fprintf(stderr, "\ttoInt()::No number detected, ...


-
Overflow not detected in toInt().

Style

-
Avoid naked magic numbers. Consider

// for (int i = 31; i >= 0; i--)
#define INT_WIDTH  32
for (int i = INT_WIDTH - 1; i >= 0; i--)


-
int toInt() returns only 2 different int. Consider bool.

Minor

-
See little value in excessive in vertical spacing throughout code. Example: The need for a blank line between buys little clarity for the price of a line.

printBinary(number);

state = 0;


-
Consider parens ()

// if (number >> i & 1 == 1)
if ((number >> i) & 1 == 1)
// or simply
if ((number >> i) & 1)

Code Snippets

// Why 31?
for (int i = 31; i >= 0; i--)
// ... (int number)
... (unsigned number)
... 
if (number >> i & 1 == 1)
*num = 10* *num + param[i] - '0';
// printf(": ");
fputs(": ", stdout);
// or (2nd choice)
printf("%s", ": ");
int main(int argc, char *argv[]) {   
  if (argc > 1) return handleProgram(argv[1]);
  else ...

Context

StackExchange Code Review Q#126386, answer score: 2

Revisions (0)

No revisions yet.