patterncppMinor
Decimal/binary/hex converter
Viewed 0 times
hexdecimalbinaryconverter
Problem
I've finally finished my converter and have determined that it works, but I'm trying to make it cleaner and/or more practical (primarily with the
Are there other methods of performing these conversions that would use less code, or are these methods good enough? I am also using
```
#include
#include
typedef unsigned long long uInt64;
uInt64 calculatePower(uInt64, uInt64);
uInt64 binaryToDecimal(std::string);
uInt64 hexadecimalToDecimal(std::string);
std::string decimalToBinary(uInt64);
std::string binaryToHexadecimal(std::string);
int main()
{
std::string binary, hex;
uInt64 decimal;
uInt64 choice;
std::cout Binary & Hex (1)\n";
std::cout Decimal & Hex (2)\n";
std::cout Binary & Decimal (3)\n\n";
std::cin >> choice;
if (choice == 1)
{
std::cout Decimal Input: ";
std::cin >> decimal;
std::string binaryOutput = decimalToBinary(decimal);
std::string hexOutput = binaryToHexadecimal(binaryOutput);
std::cout Binary Input: ";
std::cin.ignore();
std::getline(std::cin, binary);
uInt64 decimalOutput = binaryToDecimal(binary);
std::string hexOutput = binaryToHexadecimal(binary);
std::cout Hex Input: ";
std::cin.ignore();
std::getline(std::cin, hex);
uInt64 decimalOutput = hexadecimalToDecimal(hex);
std::string binaryOutput = decimalToBinary(decimalOutput);
std::cout 0)
{
remainder = decimal % 2;
if (remainder == 0)
binary += '0';
else if (remainder == 1)
binary += '1';
decimal /= 2;
}
for (iter = binary.rbegin(); iter != binary.rend(); iter++)
{
switch statements). I could probably put more things into functions, but it'll be needless unless I can replace the switch statements with something simpler.Are there other methods of performing these conversions that would use less code, or are these methods good enough? I am also using
unsigned long long so that I can work with large numbers, but I can always change that if it may not be the best thing to do.```
#include
#include
typedef unsigned long long uInt64;
uInt64 calculatePower(uInt64, uInt64);
uInt64 binaryToDecimal(std::string);
uInt64 hexadecimalToDecimal(std::string);
std::string decimalToBinary(uInt64);
std::string binaryToHexadecimal(std::string);
int main()
{
std::string binary, hex;
uInt64 decimal;
uInt64 choice;
std::cout Binary & Hex (1)\n";
std::cout Decimal & Hex (2)\n";
std::cout Binary & Decimal (3)\n\n";
std::cin >> choice;
if (choice == 1)
{
std::cout Decimal Input: ";
std::cin >> decimal;
std::string binaryOutput = decimalToBinary(decimal);
std::string hexOutput = binaryToHexadecimal(binaryOutput);
std::cout Binary Input: ";
std::cin.ignore();
std::getline(std::cin, binary);
uInt64 decimalOutput = binaryToDecimal(binary);
std::string hexOutput = binaryToHexadecimal(binary);
std::cout Hex Input: ";
std::cin.ignore();
std::getline(std::cin, hex);
uInt64 decimalOutput = hexadecimalToDecimal(hex);
std::string binaryOutput = decimalToBinary(decimalOutput);
std::cout 0)
{
remainder = decimal % 2;
if (remainder == 0)
binary += '0';
else if (remainder == 1)
binary += '1';
decimal /= 2;
}
for (iter = binary.rbegin(); iter != binary.rend(); iter++)
{
Solution
system is bad. Either use
The
Your input reading should be checked. For example,
If you're thinking you're ok because choice is checked to be either 1, 2, or 3, you're not ok. You're just almost certainly ok. If the read fails, choice will still have an indeterminate value. That could mean 1, 2, or 3.
You can basically replace all of your
Similarly,
If you're not willing to do that, but you're willing to assume that '0'...'9' is always contiguous, you can use the class
As an example, you could simplify your beastly
When arguments are NOT modified, they should be passed as
I tend to put arguments names in declarations. In this situation, it's a marginal concern since all of the functions have very obvious parameters, but in some cases, it can be quite confusing to see a declaration and wonder what the different params are.
Unless performance is tight, I'd be tempted to implement everything in terms of decimal. For example,
Speaking of performance, if you wanted to, you could inline the exponent calculations instead of recalculating it from scratch every time.
(And if you're really paranoid about performance, you'll want that
If for some odd reason you don't want to do that,
As remaining can only be 0 <= remainder
Example:
getchar(), set your IDE to pause after execution, or use a terminal when running a terminal program.The
typedef for uint64 should just be uint64_t from stdint.h.Your input reading should be checked. For example,
std::cin >> choice; can fail. It should beif (!(std::cin >> choice)) { /* handle this */ }If you're thinking you're ok because choice is checked to be either 1, 2, or 3, you're not ok. You're just almost certainly ok. If the read fails, choice will still have an indeterminate value. That could mean 1, 2, or 3.
You can basically replace all of your
ToDecimal functions with strtoul (C >= C89) or strtoull (C >= C99). An unsigned long is only gauranteed to be 4 bytes, so you'll want the strtoull version.Similarly,
sprintf() can convert to hex for you (format llX).If you're not willing to do that, but you're willing to assume that '0'...'9' is always contiguous, you can use the class
value = c - '0' shortcut.As an example, you could simplify your beastly
hexidecimalToDecimal():uint64_t hexadecimalToDecimal(const std::string& binary)
{
uint64_t decimal = 0;
uint64_t power = 1;
std::string::const_reverse_iterator iter;
for (iter = binary.rbegin(); iter != binary.rend(); iter++)
{
const char ch = std::tolower(*iter);
if (ch >= 'a') {
decimal += (ch - 'a') * power;
} else {
decimal += (ch - '0') * power;
}
power *= 16;
}
return decimal;
}When arguments are NOT modified, they should be passed as
const references. This avoids the overhead of copying. (And it's part of a larger concept called const-correctness.)I tend to put arguments names in declarations. In this situation, it's a marginal concern since all of the functions have very obvious parameters, but in some cases, it can be quite confusing to see a declaration and wonder what the different params are.
Unless performance is tight, I'd be tempted to implement everything in terms of decimal. For example,
binaryToHex(bin): decimalToHex(binaryToDecimal(bin))Speaking of performance, if you wanted to, you could inline the exponent calculations instead of recalculating it from scratch every time.
uint64_t binaryToDecimal(const std::string& binary)
{
uint64_t decimal = 0;
uint64_t p = 1;
std::string::const_reverse_iterator iter;
for (iter = binary.rbegin(); iter != binary.rend(); iter++)
{
if (*iter == '1')
decimal += p;
p *= 2;
}
return decimal;
}(And if you're really paranoid about performance, you'll want that
iter++ to be ++iter as it avoid a potential copy)If for some odd reason you don't want to do that,
calculatePower() can be optimized in many different ways:- Use the
doubleversion and just cast back touint64_t.
- Use powers of 2 and abuses of shifting (x16 =
x
- Use divide and conquer style
remainder = decimal % 2;
if (remainder == 0)
binary += '0';
else if (remainder == 1)
binary += '1';As remaining can only be 0 <= remainder
<= 1, the else if is unnecessary. I would just use:
if (decimal % 2 == 0) {
binary += '0';
} else {
binary += '1';
}
You can use std::reverse (from `) rather than your manual reversal loop.Example:
std::string decimalToBinary(uint64_t decimal)
{
std::string binary;
while (decimal > 0)
{
if (decimal % 2 == 0)
binary += '0';
else
binary += '1';
decimal /= 2;
}
std::reverse(binary.begin(), binary.end());
return binary;
}Code Snippets
if (!(std::cin >> choice)) { /* handle this */ }uint64_t hexadecimalToDecimal(const std::string& binary)
{
uint64_t decimal = 0;
uint64_t power = 1;
std::string::const_reverse_iterator iter;
for (iter = binary.rbegin(); iter != binary.rend(); iter++)
{
const char ch = std::tolower(*iter);
if (ch >= 'a') {
decimal += (ch - 'a') * power;
} else {
decimal += (ch - '0') * power;
}
power *= 16;
}
return decimal;
}uint64_t binaryToDecimal(const std::string& binary)
{
uint64_t decimal = 0;
uint64_t p = 1;
std::string::const_reverse_iterator iter;
for (iter = binary.rbegin(); iter != binary.rend(); iter++)
{
if (*iter == '1')
decimal += p;
p *= 2;
}
return decimal;
}remainder = decimal % 2;
if (remainder == 0)
binary += '0';
else if (remainder == 1)
binary += '1';if (decimal % 2 == 0) {
binary += '0';
} else {
binary += '1';
}Context
StackExchange Code Review Q#25381, answer score: 5
Revisions (0)
No revisions yet.