First things first, props to you for learning good old C in the day of dynamically typed high level languages! Now on to the review...
Error checking
Right now, there's nothing to stop me from entering 512351 as the seventh digit, for example. As a rule of thumb, if something comes from user input, double-check it. Your user is the greatest threat to program integrity. For example, check the input in a loop and only save it once it is a valid bit (0 or 1), if it's not valid, simply ask your user again. But this takes me to the next issue...
Input
Right now, you ask for each bit separately. I'm lazy. I wouldn't want to have 16 keystrokes to enter 8 digits. So, instead of asking for each bit on its own, consider just asking your user to type the digits as a string. Instead of eight integer variables, you could store them (temporarily, that is) in an array of characters. See the following snippet:
char input[8 + 1 + 1]; // 8 characters + newline + null terminator
printf("Enter a binary number: ");
fgets(input, sizeof input, stdin);
input[8] = '\0'; // We don't want the newline
This reads a line from standard input and stores the first 9 characters - 8 bits and a newline - in the array input
, as well as a '\0'
to terminate the string.
Now this is way shorter!
Conversion to decimal
But now that you have your digits as characters in a array, the conversion has to change. You can even be fancy and make your conversion more generic, in case you want to handle arbitrarily long numbers one day.
A typical idiom to get the numeric value from a character is int number = char - '0';
This subtracts the ASCII value for the character 0 from the character you have, leaving for example '5'
as the integer 5. This will get strange when you use non-digit characters, of course. But you've checked that beforehand, I hope!
What's left is looping over your characters and adding their digit values to the total value.
int decimal_value = 0;
for (int i = 0; i != 8; ++i) {
int digit = input[i] - '0';
int factor = pow(2, 7 - i);
decimal_value += digit * factor;
}
But since your binary digits are always either 0 or 1, you could tweak this a bit to prevent adding 0 a bunch of times.
int decimal_value = 0;
for (int i = 0; i != 8; ++i) {
if (input[i] - '0') {
decimal_value += pow(2, 7 - i);
}
}
If the current character processed is '0'
, input[i] - '0'
will evaluate to 0, which is falsy - the body of the if
will not be executed. Otherwise, it is non-zero, which leads to the if
-body being executed. Nice!
Conversion to hex
Right now, you do two things which should not be done together. You store the hex-digits as integer value, but once they're larger than 10 you store a character instead. This... works, but it's unusual and not that great. Instead, convert both two characters! The actual conversion is left as an exercise for you, but once you have both digits stored as first_hexdigit
and last_hexdigit
for example, you could to this:
char hex[2]; // Since we have two digits, let's store those as an array.
// Note that, in contrast to before with input, we do not
// have to make the array one larger than it needs to be,
// as we do not use fgets which appends an extra character.
hex[0] = first_hexdigit >= 10 ? first_hexdigit - 10 + 'A'
: first_hexdigit + '0';
hex[1] = last_hexdigit >= 10 ? last_hexdigit - 10 + 'A'
: last_hexdigit + '0';
If you have never used it before, the ?
and :
are the ternary operator. Basically, a ? b : c
means: if a
is a truthy value, then return b, else c. Or, in other words:
int result;
if (a) {
result = b;
} else {
result = c;
}
To convert from a numeric value to a character, we switch our previous idiom around a bit: instead of subtracting our lowes character, we add it as value. If last_hexdigit
is 6, for example, this becomes 6 + '0'
which is just '6'
. But if it's 12, the computation is 12 - 10 + 'A'
, which is 2 + 'A'
which is 'C'
.
Output
Since you've stored your results a bit differently, your way of printing has to be changed as well!
Assume we have our decimal value stored, as integer, in decimal_value
and our hexadecimal as array of two characters in hexadecimal_value
.
int decimal_value; // Your decimal
char hexadecimal_value[2]; // Your hexadecimal
printf("Decimal: %d\n", decimal_value);
printf("Hexadecimal: %c%c\n", hexadecimal_value[0], hexadecimal_value[1]);
This could made better a bit if you do make the hexadecimal_value
array one larger than it needs to be and set the last character to '\0'
. Because then, hexadecimal_value
would be a string (as strings need to be null-terminated), which you could easily print with printf
:
printf("Hexadecimal: %s\n", hexadecimal_value);
Encapsulation
Now that the actual code is cleaned up a bit, what can we do to make it more beautiful on a higher level?
Exctract functionality into functions, of course!
Right now, all your code is in main
. This is okay for small programs like this, but should still be avoided. So instead, let's make some smaller functions for simple tasks! Your control flow is much like the following:
int main() {
get_input();
process_input();
print_output();
}
Since it makes sense to handle the input and output in main
(for this scale at least), let's focus on making the process_input()
!
I suggest you use three functions, which carry your main logic:
int is_valid_binary(const char *string);
char *binary_to_decimal(const char *string);
char *binary_to_hexadecimal(const char*string);
is_valid_binary
takes in a string and returns 0
if it is not a valid binary number (e.g. contains characters other than '1'
and '0'
), otherwise, it returns 1
. binary_to_decimal
takes in a string of binary digits and returns a string of the very same number, but with decimal digits. binary_to_hexadecimal
works similar; it takes a string and returns a string of hexadecimal digits. I leave actually writing these functions to you.
Once you have those three functions, your main would thus become:
int main() {
char input[8 + 1 + 1];
printf("Enter a binary number: ");
do {
fgets(input, sizeof input, stdin);
input[8] = '\0';
} while (!is_valid_binary(input));
char *decimal = binary_to_decimal(input);
char *hexadecimal = binary_to_hexadecimal(input);
printf("Decimal: %s\nHexadecimal: %s\n", decimal, hexadecimal);
// Since you allocated the memory for decimal end hexadecimal yourself,
// you have to free it yourself as well! At least, you should allocate
// it yourself.
free(decimal);
free(hexadecimal);
}
Exercise to you
To recap, consider the final version of main
I pasted above. Your task now is to write the functions is_valid_binary
, binary_to_decimal
and binary_to_hexadecimal
. You could also expand those functions to handle arbitrarily long binary numbers (not necessarily eight digits, but also 2 digits or 24 digits!). And, as an extra, take the input from the commandline. Use argv
for that one.
Addendum
As Toby Speight mentioned in the comments, it is not really a good idea to use pow()
when calculating 2^n
. First of all, pow()
uses doubles
, which adds some conversion overhead (and, worst case scenario, wrong results due to the way double
works. But it's rather unlikely.), and second: it's slow. Let me introduce you to bitmagic!
Consider the number 4, or 0b0100
. 4 times 2 is 8, or 0b1000
. There is a pattern going on here which is everywhere: if you multiply any number by 2, its binary representation gets shifted by 1 to the left. Since exponention is just repeated multiplication, 2^7
is just 1 * 2 * 2 * 2 * 2 * 2 * 2 * 2
. That's seven left-shifts right there! In C, performing left-shifts is done by using the <<
operator.
So, instead of calculating 2^7
by using the slow pow()
, you can use 1 << 7
instead!
Basically: pow(2,x) == 1 << x
.