7
\$\begingroup\$

I started a few weeks ago to learn C++ online. I just made a little binary → decimal program. Here is my code:

#include <iostream>

using namespace std;

int main()
{
    int i,j=1;
    long long decimal=0, powerOfTwo=1;
    cin>>i;
    char binary[i];
    i*=2;
    while (i)
    {
        if (i > j)
        {
           cin>>binary[j];
           ++j;
        }
        else
        {
            cout<<binary[i];
            if (i%4 == 1) //adds a space after 4 numbers (eg. 1011 0111 instead of 10110111)
                cout<<" ";
            if (binary[i] == '1')
                decimal += powerOfTwo;
            powerOfTwo *= 2;
        }
        --i;
    }
    cout<<"in ten base is equal to "<<decimal;
    return 0;
}

So I want to know, am I heading in the right direction? Please let me know what I am not doing good or any kind of suggestion.

\$\endgroup\$
  • 1
    \$\begingroup\$ The only "C++" here is cin, cout, using namespace std;. \$\endgroup\$ – coderodde Oct 22 '17 at 10:14
  • \$\begingroup\$ Can you be more specific ? Should i start with C language ? \$\endgroup\$ – Undyne Oct 22 '17 at 10:18
  • \$\begingroup\$ @Undyne It depends. Note that we can always write code. However, if you want to become more productive, it makes sense to use standard library. There is a saying: good programmers program, best programmers reuse. \$\endgroup\$ – coderodde Oct 22 '17 at 10:28
  • \$\begingroup\$ @Undyne Do you use a book, or online examples to learn C++? \$\endgroup\$ – Zeta Oct 22 '17 at 10:56
  • \$\begingroup\$ @Zeta I'm learning from here \$\endgroup\$ – Undyne Oct 22 '17 at 12:03
15
\$\begingroup\$

What would Bob do?

You've written an interactive program, but the user does not know what your program does. Therefore, Bob—the generic user—might accidentally write "test" instead of a number, and suddenly your program runs wild.

A first step in the right direction would be to tell the user what your program will do—as long as its interactive. Note that this is not C++ specific.

We can do so by starting with:

cout << "This program converts a binary number to a decimal one\n"
        "Please state the length of your number: ";

But that's error prone. First of all, Bob might not know how long the binary number is. Try to check whether 1101011010101101 is 14 or 15 characters long. That's a trick question, those are 16 characters.

So instead, just let Bob write anything and then check how long it is and whether it was really a binary number.

Prefer std::string to char[N]

Instead of char binary[i], we will use a std::string binary. This makes the input a lot easier. However, to handle the input easier, let's write a function. Actually, let's write two:

bool is_binary_number(const std::string& binary){
    return std::all_of(binary.begin(), binary.end(), [](char c){
        return c == '0' || c == '1';
    });
}

That's using C++11's all_of in combination with a lambda. In case you don't know that yet, here's the same variant written in a range-based for loop:

bool is_binary_number(const std::string& binary){
    for(char c : binary) {
        if(c != '0' && c != '1') {
            return false;
        }
    }
    return true;
}

But this is C++11 syntax. In case you're not familar with C++11 at all, it's almost the same as

bool is_binary_number(const std::string& binary){
    for(size_t i = 0; i < binary.size(); ++i) {
        if(binary[i] != '0' && binary[i] != '1') {
            return false;
        }
    }
    return true;
}

In case you don't know std::string & yet, that's a reference. A reference is an alias to an already existing object. A const reference is an alias where we cannot change the value:

int        value = 15;
int&       ref   = value;
const int& cref  = value;

ref = 12;

std::cout << value << std::endl; // prints 12, since we changed it via ref
std::cout << cref  << std::endl; // prints 12, since we changed it via ref

// does not compile:
cref = 7;           // error; this reference is read only!

The details are a little bit more complicated, but in the end, when we call is_binary_number(on_a_string), we won't make a copy of on_a_string.

Either way, now that we have a function to check whether a string is a binary number, we can write our function to ask Bob for input:

std::string ask_binary_number() {
    std::cout << "Please enter a binary number: ";

    while(true) {
        std::string input;
        std::cin >> input;

        if(is_binary_number(input)) {
            return input;  
        } else {
            std::cout << "That wasn't a binary number, try again: ";
        }
    }
}

We can now use this in main:

#include <iostream>
#include <string>        // necessary for std::string

int main () {
    std::string binary = ask_binary_number();

    ...
}

Note that we don't need to touch main at all if we want to change the input method later on. We only need to change ask_binary_number. This makes it easy to ask the user for another number, or re-use ask_binary_number in another project.

Exercise: Usually, you don't want leading zeroes in binary numbers. Which function do you have to change to not allow leading zeroes in the result of ask_binary_number? What changes are necessary?

Keep it simple and stupid

In your for loop, you do two things at once: you print the binary number and convert the number to a decimal. Unless you know that this is a bottleneck in your program and you need cache locality or something similar to keep your code fast, do one thing and one thing only. This makes it a lot easier to change the code later without breaking something else.

We can split this functionality again into two functions:

void print_binary_with_spaces(const std::string & binary, size_t digits = 4){
    // exercise; almost solved by your own code
}

unsigned long binary_to_decimal(const std::string & binary){
    // exercise; almost solved by your own code
}

If you implement all functions given above, you end up with the following main:

#include <iostream>
#include <string>

// functions here (either completely or only their declaration)

int main() {
    std::cout << "This program converts a binary number to a decimal one." << std::endl;
    std::string binary = ask_binary_number();

    std::cout << "Your binary number ";

    print_binary_with_spaces(binary);

    std::cout << " is " << binary_to_decimal(binary) << "in decimal"
              << std::endl;
}

Note that all complex interactions are moved into functions. However, as you have seen in ask_binary_number and is_binary_number those functions aren't complex themselves either. Keeping things simple is a principle (KISS) that's often applied, as is splitting the functionality (separation of concerns).

A concrete example of overly complex code

Why am I pointing to simplicity? Because your while loop is overly complex:

i = i * 2;
j = 1;
while (i)
{
    if (i > j)        
    {
       ...            // lets call this A
       ++j;
    }
    else
    {
        ...          // lets call this B
    }
    --i;
}

Essentially, you have the following loops:

for(j = 0; j < i; ++j)
{
    A; // see above
}

for(--i; i > 0; --i)
{
    B; // see above
}

If we used that in your program, it would look like this:

for(int j = 0; j < i; ++j){
    cin >> binary[j];
}

while(i-- > 0){
    cout << binary[i];
    if (i%4 == 1) 
        cout << " ";
    if (binary[i] == '1')
        decimal += powerOfTwo;
    powerOfTwo *= 2;
}

This is not only easier to understand, it's even shorter. So try to keep your loops simple.

\$\endgroup\$
  • \$\begingroup\$ Instead of is_binary_number, you can use std::all_of. Always prefer a standard library algorithm if there is one :) \$\endgroup\$ – Rakete1111 Oct 22 '17 at 11:06
  • 1
    \$\begingroup\$ @Rakete1111 that would use a lambda, and I'm not sure our beginner is ready for that one, yet the <algorithm> header at all. Always prefer the code that's easier to understand. \$\endgroup\$ – Zeta Oct 22 '17 at 11:20
  • 1
    \$\begingroup\$ @Zeta Thanks, I understand like 4/5 of the syntax, but you really helped me learn how to organize my code. I'll come back with the full code later. Thanks again ! \$\endgroup\$ – Undyne Oct 22 '17 at 11:42
  • 1
    \$\begingroup\$ @Undyne: You're allowed to ask for follow-up reviews. That being said, now to string vs string&: the latter is a reference. A reference is an alias to an already existing object/function/value. Your site handles them here. On the other question: they should have the same type. Your compiler might warn you about that if you enable (more) warnings. In C++14 you can use auto to deduce the return type automatically (learncpp.com/cpp-tutorial/4-8-the-auto-keyword). \$\endgroup\$ – Zeta Oct 22 '17 at 13:08
  • 1
    \$\begingroup\$ @Undyne I've added some examples to the section regarding references. \$\endgroup\$ – Zeta Oct 22 '17 at 13:19
0
\$\begingroup\$

Try this instead strtoul library method in cstdlib http://www.cplusplus.com/reference/cstdlib/strtoul/

from this you can convert any radix base in string format to unsigned long long number.

usage-example here :

/* strtoul binary-to-decimal example */

#include<iostream>
#include<cstring>
#include<cstdint>
#include<cstdlib>

#define BINARY_BASE 2 /*Defining binary base*/
#define OCTAL_BASE 8  /*Defining octal base*/
#define DECIMAL_BASE 10 /*Defining decimal base*/
#define HEXA_BASE 16    /*Defining hexa-decimal base*/
#define BASE32_BASE 32 /*Defining base32 base*/

bool isValidNumber4Base(const char* numStr,int base)
{

    const char *validBinary = "01";
    const char *validOctal = "01234567";
    const char *validDecimal = "0123456789";
    const char *validHex = "0123456789abcdefxABCDEFX";
    const char *validBase32 = "0123456789abcdefghijklmnopqrstuvABCDEFGHIJKLMNOPQRSTUV";
    const char *validNumber = NULL;

    validNumber = (base == BINARY_BASE) ? validBinary : ((base == OCTAL_BASE) ? validOctal :
                    (base == DECIMAL_BASE) ? validDecimal : (base == HEXA_BASE) ? validHex : (base == BASE32_BASE) ? validBase32 : NULL);

    if(validNumber ==  NULL)
    {
        std::cerr<<"Invalid base encountered"<<std::endl;
        exit(EXIT_FAILURE);
    }

    return (!numStr[strspn(numStr,validNumber)]) ? true : false;
}

uint64_t getDecimal4mBinary(const char *binaryStr)
{
    if (isValidNumber4Base(binaryStr,BINARY_BASE))
    {
        char *endBinaryStr;
        return strtoull(binaryStr,&endBinaryStr,BINARY_BASE);
    }

    else
    {
        std::cerr<<"Invalid binary-number encountered"<<std::endl;
        exit(EXIT_FAILURE);
    }
}

/*Test Method */
int main ()
{
  char *binary_str = "10000001";
  uint64_t dec4mBinary = getDecimal4mBinary(binary_str);    
  std::cout<<"Decimal equivalent\n"<<dec4mBinary<<std::endl;        
  return 0;
}

Code Working on C++ 14 compiler : https://ideone.com/CIOkUM

C version of bin-to-decimal and other binary related stuff could be found here. https://github.com/haseeb-heaven/BinaryLibrary4C/blob/master/binary4c.h

\$\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.