Decimal to Binary converter

Question

This program might seem lazy, I also tried iterating through the vector but performance was poor so I used this lazy approach that has more performance

#ifndef BINARY_HH
#define BINARY_HH
/**************************************************************************
 * Name:  Binary.hh
 * Author: Samuel Oseh
 * Date: 27/09/2020
 * File Info: This file contains class method-function prototypes
 * Purpose: Binary Converter is a program that converts a decimal
 *          to its binary equivalents and also performs other calculations
 **************************************************************************/

#include <vector>
#include <string>

class Binary {
    public:
        Binary( const int &, const char & );
        ~Binary() {}
        Binary( const Binary & );
        void printBinary() const;
        void toBinary();
        char getType() const;
        int getDecimal() const;
        Binary &operator+=( const Binary &binaryToAdd );
        Binary &operator-=( const Binary &binaryToSub );
        Binary &operator*=( const Binary &binaryToMul );
        Binary &operator/=( const Binary &binaryToDiv );
        bool operator==( const Binary &binaryToCompare ) const;
        inline bool operator!=( const Binary &binaryToCompare ) const {
            return !( *this == binaryToCompare );
        }
        const Binary &operator=( const Binary &binaryToCopy );
    private:
        char type;
        int decimal;
        std::vector< int > binary{};
};

#endif

/**************************************************************************
 * Name:  Binary.cc
 * Author: Samuel Oseh
 * Date: 27/09/2020
 * File Info: This file contains class method-function definitions
 * Purpose: Binary Converter is a program that converts a decimal
 *          to its binary equivalents and also performs other calculations
 **************************************************************************/
#include <iostream>
#include <stdexcept>
#include "Binary.hh"

Binary::Binary( const int &d, const char &t ) {
    if ( tolower( t ) == 'd' )
        type = t;
    else
        throw std::invalid_argument( "type must be  'd' only." );
    if ( d < 0 )
        throw std::invalid_argument( "decimal value must be greater than 0." );
    decimal = d;
}

Binary::Binary( const Binary &binaryToCopy ) {
    decimal = binaryToCopy.decimal;
    type = binaryToCopy.type;
    for ( unsigned int counter = 0; counter < binaryToCopy.binary.size(); ++ counter ) {
        binary.push_back( binaryToCopy.binary[ counter ] );
    }
}

void Binary::toBinary() {
    if ( type == 'd' ) {
        int val = decimal;

        while ( val != 0 ) {
            binary.insert( binary.begin(), val % 2 );
            val /= 2;
        }
    }
    else {
        throw std::invalid_argument( "Invalid type conversion" );
    }
}

void Binary::printBinary() const {
    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot print binary object without converting it" );
    for ( int number : binary ) {
        std::cout << number << "";
    }
}

char Binary::getType() const {
    return type;
}

int Binary::getDecimal() const {
    return decimal;
}

Binary &Binary::operator+=( const Binary &binaryToAdd ) { 
    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );
    if ( binaryToAdd.binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );

    int decimalSum = decimal + binaryToAdd.decimal;
    Binary *b = new Binary( decimalSum, 'd' );
    b->toBinary();
    *this = *b;
    delete b;
    return *this;
}

Binary &Binary::operator-=( const Binary &binaryToAdd ) { 
    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );
    if ( binaryToAdd.binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );

    int decimalSum = decimal - binaryToAdd.decimal;
    if ( decimalSum < 0 ) {
        throw std::invalid_argument( "Can not perform subtraction from a lesser binary" );
    }

    Binary *b = new Binary( decimalSum, 'd' );
    b->toBinary();
    *this = *b;
    delete b;
    return *this;
}

Binary &Binary::operator*=( const Binary &binaryToMul ) { 
    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );
    if ( binaryToMul.binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );

    int decimalSum = decimal * binaryToMul.decimal;
    Binary *b = new Binary( decimalSum, 'd' );
    b->toBinary();
    *this = *b;
    delete b;
    return *this;
}

Binary &Binary::operator/=( const Binary &binaryToDiv ) { 
    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );
    if ( binaryToDiv.binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );

    int decimalSum = decimal / binaryToDiv.decimal;
    Binary *b = new Binary( decimalSum, 'd' );
    b->toBinary();
    *this = *b;
    delete b;
    return *this;
}

bool Binary::operator==( const Binary &binaryToCompare ) const {
    if ( decimal == binaryToCompare.decimal )
        return true;
    return false;
}
const Binary &Binary::operator=( const Binary &binaryToCopy ) {
    decimal = binaryToCopy.decimal;
    type = binaryToCopy.type;
    binary.clear();
    for ( unsigned int counter = 0; counter < binaryToCopy.binary.size(); ++ counter ) {
        binary.push_back( binaryToCopy.binary[ counter ] );
    }
    return *this;
}

Answer 1

Overview

I think the most telling part is this:

        int decimal;

That's probably not a decimal number. In most modern computer this is stored as a binary number (though in some exceedingly old computers they did use decimals but they gave up on decimal computers in favor of binary very quickly).

The problem with your code is that a number is a number. Its base is only important when you visualize it, parse it or store it. Storing the number is an abstraction that the hardware layer has taken care of (the hardware knows how to store/and perform basic operation on integers). So the only thing to you is the presentation layer (printing and parsing).

Decimal 10:
Binary  1010
Hex     0xA

These are all simply the visual representation of the same concept. We already have the concept of an integer number provided by the language int represents a value.

---

Code Review

The type t parameter is superfluous here.

Binary::Binary( const int &d, const char &t ) {

The value d is a number already. You only need the type if you are converting from a visual representation (i.e. a string). Otherwise it is always a number.

---

Not sure why you don't support negative values:

    if ( d < 0 )
        throw std::invalid_argument( "decimal value must be greater than 0."     

Its not that hard?

---

This could be simlified:

    for ( unsigned int counter = 0; counter < binaryToCopy.binary.size(); ++ counter ) {
        binary.push_back( binaryToCopy.binary[ counter ] );
    }

    // Simpler to write:

    binary = binaryToCopy.binary;

---

If you want to store locally an easy to convert to text representation you could use std::bitset<32>.

std::bitset<32> binary;
void Binary::toBinary() {
    binary = decimal;  // converts it into a bitset.
                       // bitsset when put on a stream will show 1/0
                       // Note internally it is just an int.
}

---

Sure it is nice to have a print function.

void Binary::printBinary() const {

But in C++ we normally use operator<< to place data on an output stream. So it is nice to write the appropriate function that does that. Not it can simply call the printBinary() function. But you will need to modify your print function to accept a stream (you can default it to std::out for backwards compatability.

    void Binary::printBinary(std::ostream& str = std::cout) const {
        binary.toBinary(); // You can convert see next section.
        out << binary;     // Assuming you used std::bitset.
    }
    friend std::ostream& operator<<(std::ostream& str, Binary const& b)
    {
        b.printBinary(str);
        return str;
    }

---

You can convert the number in a const method.

    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot print binary object without converting it" );

The binary object is storing temporary state (i.e it does not represent the state of the object. It represents an easy printable value of the current state. So this is a perfect place to mark a member as mutable.

    mutable std::bitset<32>    binary;

Now you can update the state in a const function:

---

Once the value has been converted to a number its type is irelavant.

char Binary::getType() const {
    return type;
}

---

You are not returning a decimal. You are returning a number.

int Binary::getDecimal() const {
    return decimal;
}

Its actually stored as binary you know.

---

Wow this seems like a lot of work.

Binary &Binary::operator+=( const Binary &binaryToAdd ) { 
    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );
    if ( binaryToAdd.binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );

    int decimalSum = decimal + binaryToAdd.decimal;
    Binary *b = new Binary( decimalSum, 'd' );
    b->toBinary();
    *this = *b;
    delete b;
    return *this;
}

First lets not create dynamic types:

    Binary *b = new Binary( decimalSum, 'd' );
    b->toBinary();
    *this = *b;
    delete b;

This can be simplifies to:

    Binary  b( decimalSum, 'd' );
    b.toBinary();
    *this = b;

But why do all that why not simplify one more step and remove the intermediate object.

    decimal += binaryToAdd.decimal;
    binary.clear();   // Not sure if you need this but probably.
    toBinary();

Not sure why they need to be already converted?

    if ( binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );
    if ( binaryToAdd.binary.size() == 0 ) 
        throw std::invalid_argument( "Cannot add binary object without converting it" );

Just get rid of that.

---

Same thing applies here:

Binary &Binary::operator-=( const Binary &binaryToAdd ) { 
Binary &Binary::operator*=( const Binary &binaryToMul ) { 
Binary &Binary::operator/=( const Binary &binaryToDiv ) { 
   //  ^ Note in C++ (unlike C) the & and * go with the type (traditionally).

---

You implemented the += family of operators. This makes the next step so simply. I am surprised you did not implement the + family of operators.

Binary Binary::operator+(Binary const& rhs) const { 
    Binary newValue(*this);  // copy;
    return newValue += rhs;  // Add to the copy and return.
}

---

This can be simplified:

    if ( decimal == binaryToCompare.decimal )
        return true;
    return false;

To

    return decimal == binaryToCompare.decimal;

If you find your self doing:

     if (test) {
         return true;
     }
     else {
         return false;
     }

     This is the same as

     return test; // as test must be a boolean value (or convertible to cone).

---