I'm mainly looking for error checking / type safety advice.
main.cpp
#include <iostream>
#include <string>
#include <cmath>
#include <numeric>
void binaryToDecimal();
void decimalToBinary();
int main()
{
std::ios::sync_with_stdio(false);
bool running = true;
int ans;
while (running)
{
std::cout << "1. Binary to decimal\n2. Decimal to binary\n3. Quit\n";
std::cin >> ans;
std::cout << '\n';
switch (ans)
{
case 1:
binaryToDecimal();
break;
case 2:
decimalToBinary();
break;
case 3:
running = false;
break;
default:
break;
}
}
return 0;
}
void binaryToDecimal()
{
int out = 0;
std::string in;
std::cout << "Input binary number: ";
std::cin >> in;
int i = in.size() - 1;
if (i > 32)
{
std::cerr << "That number is too big!\n\n";
return;
}
for (int j = 0; i > -1; --i, ++j)
{
out += static_cast<int>(std::pow(2, i)) * (in[j] - '0');
}
std::cout << in << " when converted to decimal is " << out << "\n\n";
}
void decimalToBinary()
{
std::string out = "";
std::string in;
std::cout << "Input decimal number: ";
std::cin >> in;
if (in.size() > std::to_string(std::numeric_limits<int>::max()).size())
{
std::cerr << "That number is too big!\n\n";
return;
}
int mod = std::stoi(in);
while (mod > 0)
{
out += (mod % 2) + '0';
mod /= 2;
}
std::reverse(out.begin(), out.end());
std::cout << in << " when converted to binary is " << out << "\n\n";
}
2 Answers 2
General Observations
I'm mainly looking for error checking / type safety advice.
There doesn't seem to be a lot of error checking on user input and user input is one of the places where error checking is most important. For instance, the code doesn't handle negative numbers but there is not checking for negative input.
The code also doesn't check for lack of input, and there are no error messages when the input is wrong.
Type Safety
Since the program doesn't process negative numbers it might be better to use unsigned
or unsigned int
for conversion rather than int
.
Unnecessary Function Prototypes
If the main()
function were placed at the bottom of the file rather than the top of the file, the function prototypes
void binaryToDecimal();
void decimalToBinary();
are not needed.
Magic Numbers
There is a Magic Numbers in the binaryToDecimal()
function (32), it might be better to create symbolic constants for them to make the code more readable and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintenance easier.
Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.
In this case 32
probably refers to word size, but that is an arbitrary number, most computers these days have a 64 bit word size. You could get the actual size of the word by using the sizeof()
function and then multiplying it by 8 since sizeof()
the number of char in the object and 8 bits is the size of a char, there also might be a word size constant hidden in an include header.
Complexity
While main()
is pretty simple now, once you start adding the error checking you need it will get more complex, and it should be broken up into different function.
-
3\$\begingroup\$
std::numeric_limits<T>::digits()
might be a good choice for the maximum number of bits. en.cppreference.com/w/cpp/types/numeric_limits/digits \$\endgroup\$user673679– user6736792020年10月22日 15:43:25 +00:00Commented Oct 22, 2020 at 15:43 -
\$\begingroup\$ None of the answers to the Q&A you linked mention anything about using a
union
of a natural integer andstd::bitset
. And I would advise strongly against it. It isn't going to do what you hope... \$\endgroup\$Cody Gray– Cody Gray2020年10月22日 23:37:35 +00:00Commented Oct 22, 2020 at 23:37 -
\$\begingroup\$ @CodyGray Removed that paragraph. \$\endgroup\$2020年10月22日 23:42:15 +00:00Commented Oct 22, 2020 at 23:42
-
\$\begingroup\$ I would dispute "Unnecessary Function Prototypes" under Robert Martins step down rule. Humans like to read stories in order. \$\endgroup\$candied_orange– candied_orange2020年10月23日 09:54:57 +00:00Commented Oct 23, 2020 at 9:54
-
\$\begingroup\$ @candied_orange I think of
main()
as a special case because it is the operating system entry point. The use ofmain()
should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program. The functions thatmain()
calls are more important thanmain()
itself.main()
is the final chapter in a mystery that ties all the ends together. \$\endgroup\$2020年10月23日 12:55:20 +00:00Commented Oct 23, 2020 at 12:55
Handling invalid input
Not handling invalid input can be dangerous, especially in this case since it can cause strange outputs. A good practice is to validate the input before you perform any calculations / process it. Here's an example, imagine you opened command prompt on your laptop and entered a false statement.
This would show you unknown command x
, Instead, what if it started to spit out random numbers endlessly, and crashed in 2-3 seconds? You don't want this to happen with your programs either.
Use enum
s or enum class
s
@pacmaninbw has already written in his review the larger downsides of magic numbers. But how do you avoid this? Use an enum. The syntax is simple, and makes your code more readable for others.
enum Choices
{ bin_to_dec = 1, dec_to_bin, quit };
This will automatically set dec_to_bin
to 2
, and quit
to 3
.
Now your switch
statement is clear;
switch ( ans )
{
case bin_to_dec:
....
case dec_to_bin:
....
case quit:
...
default:
...
}
Logic
There is a much simpler way to convert decimal to binary in C++. All you have to do is to use std::bitset.
It would simplify the implementation greatly
Decimal to binary
Here is an example
void dec_to_bin()
{
int number = 736;
std::cout << (std::bitset<32>)number;
}
I have used 32
since that is the max size of signed int
.
Output:
00000000000000000000001011100000
You can even use its to_string()
method to get a string representation, which might be useful to remove the extra 0
's.
binary.erase(0, binary.find_first_not_of('0'));
std::cout << binary;
1011100000
Binary to decimal
The best part is, this works the other way around too, by using .to_ulong()
assume you have the binary in a string like this.
std::string binary = "1011100000"; // from our previous example, we know this is = 736
auto decimal = std::bitset<32>(binary).to_ulong();
std::cout << decimal;
736
Remember that validating input before doing this is a must.
Explore related questions
See similar questions with these tags.