C binary number converter (Hex and Decimals)
I'm still new to C programming, but I'm playing around with it to ensure I progress. I just made the following converter that takes 8 numbers (0 and 1) as binary code then converts the binary value to decimal and hex values. Below is the full code.
Aside from perhaps adding loops to make my code less redundant (especially the printf()
statements), is there something I could do to make my code look more professional.
#include <stdio.h>
#include <math.h>
int main() {
int b1, b2, b3, b4, b5, b6, b7, b8, decimal, hex1, hex2;
char convert1, convert2;
printf("Please enter the 1st bit of your binary number (0 or 1): ");
scanf("%i", &b1);
printf("Please enter the 2nd bit of your binary number (0 or 1) ");
scanf("%i", &b2);
printf("Please enter the 3rd bit of your binary number (0 or 1) ");
scanf("%i", &b3);
printf("Please enter the 4th bit of your binary number (0 or 1) ");
scanf("%i", &b4);
printf("Please enter the 5th bit of your binary number (0 or 1) ");
scanf("%i", &b5);
printf("Please enter the 6th bit of your binary number (0 or 1) ");
scanf("%i", &b6);
printf("Please enter the 7th bit of your binary number (0 or 1) ");
scanf("%i", &b7);
printf("Please enter the 8th bit of your binary number (0 or 1) ");
scanf("%i", &b8);
decimal = b1 * pow(2,7) + b2 * pow(2,6) + b3 * pow(2,5) + b4 * pow(2,4) + b5 * pow(2,3) + b6 * pow(2,2) + b7 * pow(2,1) + b8 * pow(2,0);
hex1 = b1 * 8 + b2 * 4 + b3 * 2 + b4 * 1;
hex2 = b5 * 8 + b6 * 4 + b7 * 2 + b8 * 1;
switch(hex1) {
case 10:
convert1 = 'A';
break;
case 11:
convert1 = 'B';
break;
case 12:
convert1 = 'C';
break;
case 13:
convert1 = 'D';
break;
case 14:
convert1 = 'E';
break;
case 15:
convert1 = 'F';
break;
}
switch(hex2) {
case 10:
convert2 = 'A';
break;
case 11:
convert2 = 'B';
break;
case 12:
convert2 = 'C';
break;
case 13:
convert2 = 'D';
break;
case 14:
convert2 = 'E';
break;
case 15:
convert2 = 'F';
break;
}
printf("nThe decimal value of your binary number is %inn", decimal);
printf("The hexadecimal value of your binary number is: ");
if(hex1 >= 10 && hex2 < 10) {
printf("%c%in", convert1, hex2);
}
else if (hex1 <10 && hex2 >= 10) {
printf("%i%cn", hex1, convert2);
}
else if (hex1 >= 10 && hex2 >=10) {
printf("%c%cn", convert1, convert2);
}
else {
printf("%i%in", hex1, hex2);
}
return 0;
}
beginner c number-systems
add a comment |
I'm still new to C programming, but I'm playing around with it to ensure I progress. I just made the following converter that takes 8 numbers (0 and 1) as binary code then converts the binary value to decimal and hex values. Below is the full code.
Aside from perhaps adding loops to make my code less redundant (especially the printf()
statements), is there something I could do to make my code look more professional.
#include <stdio.h>
#include <math.h>
int main() {
int b1, b2, b3, b4, b5, b6, b7, b8, decimal, hex1, hex2;
char convert1, convert2;
printf("Please enter the 1st bit of your binary number (0 or 1): ");
scanf("%i", &b1);
printf("Please enter the 2nd bit of your binary number (0 or 1) ");
scanf("%i", &b2);
printf("Please enter the 3rd bit of your binary number (0 or 1) ");
scanf("%i", &b3);
printf("Please enter the 4th bit of your binary number (0 or 1) ");
scanf("%i", &b4);
printf("Please enter the 5th bit of your binary number (0 or 1) ");
scanf("%i", &b5);
printf("Please enter the 6th bit of your binary number (0 or 1) ");
scanf("%i", &b6);
printf("Please enter the 7th bit of your binary number (0 or 1) ");
scanf("%i", &b7);
printf("Please enter the 8th bit of your binary number (0 or 1) ");
scanf("%i", &b8);
decimal = b1 * pow(2,7) + b2 * pow(2,6) + b3 * pow(2,5) + b4 * pow(2,4) + b5 * pow(2,3) + b6 * pow(2,2) + b7 * pow(2,1) + b8 * pow(2,0);
hex1 = b1 * 8 + b2 * 4 + b3 * 2 + b4 * 1;
hex2 = b5 * 8 + b6 * 4 + b7 * 2 + b8 * 1;
switch(hex1) {
case 10:
convert1 = 'A';
break;
case 11:
convert1 = 'B';
break;
case 12:
convert1 = 'C';
break;
case 13:
convert1 = 'D';
break;
case 14:
convert1 = 'E';
break;
case 15:
convert1 = 'F';
break;
}
switch(hex2) {
case 10:
convert2 = 'A';
break;
case 11:
convert2 = 'B';
break;
case 12:
convert2 = 'C';
break;
case 13:
convert2 = 'D';
break;
case 14:
convert2 = 'E';
break;
case 15:
convert2 = 'F';
break;
}
printf("nThe decimal value of your binary number is %inn", decimal);
printf("The hexadecimal value of your binary number is: ");
if(hex1 >= 10 && hex2 < 10) {
printf("%c%in", convert1, hex2);
}
else if (hex1 <10 && hex2 >= 10) {
printf("%i%cn", hex1, convert2);
}
else if (hex1 >= 10 && hex2 >=10) {
printf("%c%cn", convert1, convert2);
}
else {
printf("%i%in", hex1, hex2);
}
return 0;
}
beginner c number-systems
add a comment |
I'm still new to C programming, but I'm playing around with it to ensure I progress. I just made the following converter that takes 8 numbers (0 and 1) as binary code then converts the binary value to decimal and hex values. Below is the full code.
Aside from perhaps adding loops to make my code less redundant (especially the printf()
statements), is there something I could do to make my code look more professional.
#include <stdio.h>
#include <math.h>
int main() {
int b1, b2, b3, b4, b5, b6, b7, b8, decimal, hex1, hex2;
char convert1, convert2;
printf("Please enter the 1st bit of your binary number (0 or 1): ");
scanf("%i", &b1);
printf("Please enter the 2nd bit of your binary number (0 or 1) ");
scanf("%i", &b2);
printf("Please enter the 3rd bit of your binary number (0 or 1) ");
scanf("%i", &b3);
printf("Please enter the 4th bit of your binary number (0 or 1) ");
scanf("%i", &b4);
printf("Please enter the 5th bit of your binary number (0 or 1) ");
scanf("%i", &b5);
printf("Please enter the 6th bit of your binary number (0 or 1) ");
scanf("%i", &b6);
printf("Please enter the 7th bit of your binary number (0 or 1) ");
scanf("%i", &b7);
printf("Please enter the 8th bit of your binary number (0 or 1) ");
scanf("%i", &b8);
decimal = b1 * pow(2,7) + b2 * pow(2,6) + b3 * pow(2,5) + b4 * pow(2,4) + b5 * pow(2,3) + b6 * pow(2,2) + b7 * pow(2,1) + b8 * pow(2,0);
hex1 = b1 * 8 + b2 * 4 + b3 * 2 + b4 * 1;
hex2 = b5 * 8 + b6 * 4 + b7 * 2 + b8 * 1;
switch(hex1) {
case 10:
convert1 = 'A';
break;
case 11:
convert1 = 'B';
break;
case 12:
convert1 = 'C';
break;
case 13:
convert1 = 'D';
break;
case 14:
convert1 = 'E';
break;
case 15:
convert1 = 'F';
break;
}
switch(hex2) {
case 10:
convert2 = 'A';
break;
case 11:
convert2 = 'B';
break;
case 12:
convert2 = 'C';
break;
case 13:
convert2 = 'D';
break;
case 14:
convert2 = 'E';
break;
case 15:
convert2 = 'F';
break;
}
printf("nThe decimal value of your binary number is %inn", decimal);
printf("The hexadecimal value of your binary number is: ");
if(hex1 >= 10 && hex2 < 10) {
printf("%c%in", convert1, hex2);
}
else if (hex1 <10 && hex2 >= 10) {
printf("%i%cn", hex1, convert2);
}
else if (hex1 >= 10 && hex2 >=10) {
printf("%c%cn", convert1, convert2);
}
else {
printf("%i%in", hex1, hex2);
}
return 0;
}
beginner c number-systems
I'm still new to C programming, but I'm playing around with it to ensure I progress. I just made the following converter that takes 8 numbers (0 and 1) as binary code then converts the binary value to decimal and hex values. Below is the full code.
Aside from perhaps adding loops to make my code less redundant (especially the printf()
statements), is there something I could do to make my code look more professional.
#include <stdio.h>
#include <math.h>
int main() {
int b1, b2, b3, b4, b5, b6, b7, b8, decimal, hex1, hex2;
char convert1, convert2;
printf("Please enter the 1st bit of your binary number (0 or 1): ");
scanf("%i", &b1);
printf("Please enter the 2nd bit of your binary number (0 or 1) ");
scanf("%i", &b2);
printf("Please enter the 3rd bit of your binary number (0 or 1) ");
scanf("%i", &b3);
printf("Please enter the 4th bit of your binary number (0 or 1) ");
scanf("%i", &b4);
printf("Please enter the 5th bit of your binary number (0 or 1) ");
scanf("%i", &b5);
printf("Please enter the 6th bit of your binary number (0 or 1) ");
scanf("%i", &b6);
printf("Please enter the 7th bit of your binary number (0 or 1) ");
scanf("%i", &b7);
printf("Please enter the 8th bit of your binary number (0 or 1) ");
scanf("%i", &b8);
decimal = b1 * pow(2,7) + b2 * pow(2,6) + b3 * pow(2,5) + b4 * pow(2,4) + b5 * pow(2,3) + b6 * pow(2,2) + b7 * pow(2,1) + b8 * pow(2,0);
hex1 = b1 * 8 + b2 * 4 + b3 * 2 + b4 * 1;
hex2 = b5 * 8 + b6 * 4 + b7 * 2 + b8 * 1;
switch(hex1) {
case 10:
convert1 = 'A';
break;
case 11:
convert1 = 'B';
break;
case 12:
convert1 = 'C';
break;
case 13:
convert1 = 'D';
break;
case 14:
convert1 = 'E';
break;
case 15:
convert1 = 'F';
break;
}
switch(hex2) {
case 10:
convert2 = 'A';
break;
case 11:
convert2 = 'B';
break;
case 12:
convert2 = 'C';
break;
case 13:
convert2 = 'D';
break;
case 14:
convert2 = 'E';
break;
case 15:
convert2 = 'F';
break;
}
printf("nThe decimal value of your binary number is %inn", decimal);
printf("The hexadecimal value of your binary number is: ");
if(hex1 >= 10 && hex2 < 10) {
printf("%c%in", convert1, hex2);
}
else if (hex1 <10 && hex2 >= 10) {
printf("%i%cn", hex1, convert2);
}
else if (hex1 >= 10 && hex2 >=10) {
printf("%c%cn", convert1, convert2);
}
else {
printf("%i%in", hex1, hex2);
}
return 0;
}
beginner c number-systems
beginner c number-systems
edited Sep 19 '17 at 9:35
200_success
128k15152413
128k15152413
asked Sep 19 '17 at 9:23
Isaac Asante
312
312
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
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] = ''; // 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 ''
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: %dn", decimal_value);
printf("Hexadecimal: %c%cn", 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 ''
. Because then, hexadecimal_value
would be a string (as strings need to be null-terminated), which you could easily print with printf
:
printf("Hexadecimal: %sn", 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] = '';
} while (!is_valid_binary(input));
char *decimal = binary_to_decimal(input);
char *hexadecimal = binary_to_hexadecimal(input);
printf("Decimal: %snHexadecimal: %sn", 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
.
1
Good review - you might want to make it even better by mentioning something about the (in)advisability ofpow()
for raising integers to small integer powers (and demonstrate<<
as an alternative for the particular case of 2^n).
– Toby Speight
Sep 19 '17 at 17:33
Good point about using bitshifts instead ofpow
, didn't even think about that.
– Phil Kiener
Sep 19 '17 at 17:38
"fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggestchar input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a'n'
and room for the appended null character. Or why be stingy? Use a 2x bufferchar input[20];
– chux
Sep 20 '17 at 18:30
You're right, that should've beenfgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've usedsizeof
right away. But we don't needchar input[8+1+1];
- no need to store the newline.
– Phil Kiener
Sep 20 '17 at 19:01
@PhilKiener True no need to store the'n'
, but if it is not read, it remains instdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the'n'
from the buffer if needed.
– chux
Sep 20 '17 at 20:10
|
show 1 more comment
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f176043%2fc-binary-number-converter-hex-and-decimals%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
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] = ''; // 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 ''
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: %dn", decimal_value);
printf("Hexadecimal: %c%cn", 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 ''
. Because then, hexadecimal_value
would be a string (as strings need to be null-terminated), which you could easily print with printf
:
printf("Hexadecimal: %sn", 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] = '';
} while (!is_valid_binary(input));
char *decimal = binary_to_decimal(input);
char *hexadecimal = binary_to_hexadecimal(input);
printf("Decimal: %snHexadecimal: %sn", 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
.
1
Good review - you might want to make it even better by mentioning something about the (in)advisability ofpow()
for raising integers to small integer powers (and demonstrate<<
as an alternative for the particular case of 2^n).
– Toby Speight
Sep 19 '17 at 17:33
Good point about using bitshifts instead ofpow
, didn't even think about that.
– Phil Kiener
Sep 19 '17 at 17:38
"fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggestchar input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a'n'
and room for the appended null character. Or why be stingy? Use a 2x bufferchar input[20];
– chux
Sep 20 '17 at 18:30
You're right, that should've beenfgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've usedsizeof
right away. But we don't needchar input[8+1+1];
- no need to store the newline.
– Phil Kiener
Sep 20 '17 at 19:01
@PhilKiener True no need to store the'n'
, but if it is not read, it remains instdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the'n'
from the buffer if needed.
– chux
Sep 20 '17 at 20:10
|
show 1 more comment
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] = ''; // 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 ''
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: %dn", decimal_value);
printf("Hexadecimal: %c%cn", 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 ''
. Because then, hexadecimal_value
would be a string (as strings need to be null-terminated), which you could easily print with printf
:
printf("Hexadecimal: %sn", 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] = '';
} while (!is_valid_binary(input));
char *decimal = binary_to_decimal(input);
char *hexadecimal = binary_to_hexadecimal(input);
printf("Decimal: %snHexadecimal: %sn", 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
.
1
Good review - you might want to make it even better by mentioning something about the (in)advisability ofpow()
for raising integers to small integer powers (and demonstrate<<
as an alternative for the particular case of 2^n).
– Toby Speight
Sep 19 '17 at 17:33
Good point about using bitshifts instead ofpow
, didn't even think about that.
– Phil Kiener
Sep 19 '17 at 17:38
"fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggestchar input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a'n'
and room for the appended null character. Or why be stingy? Use a 2x bufferchar input[20];
– chux
Sep 20 '17 at 18:30
You're right, that should've beenfgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've usedsizeof
right away. But we don't needchar input[8+1+1];
- no need to store the newline.
– Phil Kiener
Sep 20 '17 at 19:01
@PhilKiener True no need to store the'n'
, but if it is not read, it remains instdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the'n'
from the buffer if needed.
– chux
Sep 20 '17 at 20:10
|
show 1 more comment
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] = ''; // 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 ''
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: %dn", decimal_value);
printf("Hexadecimal: %c%cn", 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 ''
. Because then, hexadecimal_value
would be a string (as strings need to be null-terminated), which you could easily print with printf
:
printf("Hexadecimal: %sn", 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] = '';
} while (!is_valid_binary(input));
char *decimal = binary_to_decimal(input);
char *hexadecimal = binary_to_hexadecimal(input);
printf("Decimal: %snHexadecimal: %sn", 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
.
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] = ''; // 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 ''
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: %dn", decimal_value);
printf("Hexadecimal: %c%cn", 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 ''
. Because then, hexadecimal_value
would be a string (as strings need to be null-terminated), which you could easily print with printf
:
printf("Hexadecimal: %sn", 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] = '';
} while (!is_valid_binary(input));
char *decimal = binary_to_decimal(input);
char *hexadecimal = binary_to_hexadecimal(input);
printf("Decimal: %snHexadecimal: %sn", 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
.
edited Sep 20 '17 at 23:32
answered Sep 19 '17 at 17:25
Phil Kiener
56528
56528
1
Good review - you might want to make it even better by mentioning something about the (in)advisability ofpow()
for raising integers to small integer powers (and demonstrate<<
as an alternative for the particular case of 2^n).
– Toby Speight
Sep 19 '17 at 17:33
Good point about using bitshifts instead ofpow
, didn't even think about that.
– Phil Kiener
Sep 19 '17 at 17:38
"fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggestchar input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a'n'
and room for the appended null character. Or why be stingy? Use a 2x bufferchar input[20];
– chux
Sep 20 '17 at 18:30
You're right, that should've beenfgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've usedsizeof
right away. But we don't needchar input[8+1+1];
- no need to store the newline.
– Phil Kiener
Sep 20 '17 at 19:01
@PhilKiener True no need to store the'n'
, but if it is not read, it remains instdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the'n'
from the buffer if needed.
– chux
Sep 20 '17 at 20:10
|
show 1 more comment
1
Good review - you might want to make it even better by mentioning something about the (in)advisability ofpow()
for raising integers to small integer powers (and demonstrate<<
as an alternative for the particular case of 2^n).
– Toby Speight
Sep 19 '17 at 17:33
Good point about using bitshifts instead ofpow
, didn't even think about that.
– Phil Kiener
Sep 19 '17 at 17:38
"fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggestchar input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a'n'
and room for the appended null character. Or why be stingy? Use a 2x bufferchar input[20];
– chux
Sep 20 '17 at 18:30
You're right, that should've beenfgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've usedsizeof
right away. But we don't needchar input[8+1+1];
- no need to store the newline.
– Phil Kiener
Sep 20 '17 at 19:01
@PhilKiener True no need to store the'n'
, but if it is not read, it remains instdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the'n'
from the buffer if needed.
– chux
Sep 20 '17 at 20:10
1
1
Good review - you might want to make it even better by mentioning something about the (in)advisability of
pow()
for raising integers to small integer powers (and demonstrate <<
as an alternative for the particular case of 2^n).– Toby Speight
Sep 19 '17 at 17:33
Good review - you might want to make it even better by mentioning something about the (in)advisability of
pow()
for raising integers to small integer powers (and demonstrate <<
as an alternative for the particular case of 2^n).– Toby Speight
Sep 19 '17 at 17:33
Good point about using bitshifts instead of
pow
, didn't even think about that.– Phil Kiener
Sep 19 '17 at 17:38
Good point about using bitshifts instead of
pow
, didn't even think about that.– Phil Kiener
Sep 19 '17 at 17:38
"
fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggest char input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a 'n'
and room for the appended null character. Or why be stingy? Use a 2x buffer char input[20];
– chux
Sep 20 '17 at 18:30
"
fgets(input, 8, stdin);
This reads 8 characters from standard input and stores them in the array input," is quite wrong. Suggest char input[8+1+1]; ... fgets(input, sizeof input, stdin);
to read 8 digits, a 'n'
and room for the appended null character. Or why be stingy? Use a 2x buffer char input[20];
– chux
Sep 20 '17 at 18:30
You're right, that should've been
fgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've used sizeof
right away. But we don't need char input[8+1+1];
- no need to store the newline.– Phil Kiener
Sep 20 '17 at 19:01
You're right, that should've been
fgets(input, 9, stdin)
to store eight characters and the null terminator. After all, that's how large the buffer is - should've used sizeof
right away. But we don't need char input[8+1+1];
- no need to store the newline.– Phil Kiener
Sep 20 '17 at 19:01
@PhilKiener True no need to store the
'n'
, but if it is not read, it remains in stdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the 'n'
from the buffer if needed.– chux
Sep 20 '17 at 20:10
@PhilKiener True no need to store the
'n'
, but if it is not read, it remains in stdin
for the next input function - important should code expand. Better to read a line of user input here and lop off the 'n'
from the buffer if needed.– chux
Sep 20 '17 at 20:10
|
show 1 more comment
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f176043%2fc-binary-number-converter-hex-and-decimals%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown