snippetcMinor
Convert integers into binary
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
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
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
-
Right shifting a negative number leads to implementation defined behavior. Consider
-
Good that code allows a leading
-
Pedantic point.
-
Avoid using
-
Do not use
-
Prefer terminating error messages to go to
-
Overflow not detected in
Style
-
Avoid naked magic numbers. Consider
-
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.
-
Consider parens
-
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 validint 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.