C binary number converter (Hex and Decimals)












6














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;
}









share|improve this question





























    6














    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;
    }









    share|improve this question



























      6












      6








      6







      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;
      }









      share|improve this question















      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






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Sep 19 '17 at 9:35









      200_success

      128k15152413




      128k15152413










      asked Sep 19 '17 at 9:23









      Isaac Asante

      312




      312






















          1 Answer
          1






          active

          oldest

          votes


















          5














          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.






          share|improve this answer



















          • 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 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












          • 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











          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
          });


          }
          });














          draft saved

          draft discarded


















          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









          5














          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.






          share|improve this answer



















          • 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 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












          • 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
















          5














          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.






          share|improve this answer



















          • 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 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












          • 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














          5












          5








          5






          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.






          share|improve this answer














          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.







          share|improve this answer














          share|improve this answer



          share|improve this answer








          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 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










          • "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










          • @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














          • 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 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












          • 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








          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


















          draft saved

          draft discarded




















































          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.




          draft saved


          draft discarded














          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





















































          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







          Popular posts from this blog

          Сан-Квентин

          8-я гвардейская общевойсковая армия

          Алькесар