8
\$\begingroup\$

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";
}
\$\endgroup\$
0
8
\$\begingroup\$

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.

\$\endgroup\$
7
  • 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 Oct 22 '20 at 15:43
  • \$\begingroup\$ None of the answers to the Q&A you linked mention anything about using a union of a natural integer and std::bitset. And I would advise strongly against it. It isn't going to do what you hope... \$\endgroup\$ – Cody Gray Oct 22 '20 at 23:37
  • \$\begingroup\$ @CodyGray Removed that paragraph. \$\endgroup\$ – pacmaninbw Oct 22 '20 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 Oct 23 '20 at 9:54
  • \$\begingroup\$ @candied_orange I think of main() as a special case because it is the operating system entry point. The use of main() 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 that main() calls are more important than main() itself. main() is the final chapter in a mystery that ties all the ends together. \$\endgroup\$ – pacmaninbw Oct 23 '20 at 12:55
4
\$\begingroup\$

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 enums or enum classs

@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

Yes it's the correct answer

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.