Floating-point binary/decimal/octal/hex converter

Question

I've finally decided to design a class for this program, but some of the code still looks messy. I'm also a bit concerned about DRY, particularly in binaryToOctalHex(). The program is looking better, but I'm still not liking what I'm doing in the aforementioned function. I don't mind refactoring that if necessary.

What should be done here?

Driver

#include <iostream>
#include "NumberSystemsConverter.h"

int main()
{
    unsigned choice;
    std::string str;

    std::cout << "\n\n(1) Binary  -> Decimal/Octal/Hex\n";
    std::cout << "(2) Decimal -> Binary/Octal/Hex\n";
    std::cout << "(3) Octal   -> Binary/Decimal/Hex\n";
    std::cout << "(4) Hex     -> Binary/Decimal/Octal\n\n";

    do
    {
        std::cout << "> Conversion #: ";
        std::cin >> choice;
    } while (choice < 1 || choice > 4);

    std::cout << "\n> Value: ";
    std::cin.ignore();

    if (choice == 1)
    {
        std::string binary;
        getline(std::cin, binary);
        NumberSystemsConverter conversion(binary);
        conversion.display();
    }
    else if (choice == 2)
    {
        d32 decimal;
        std::cin >> decimal;
        NumberSystemsConverter conversion(decimal);
        conversion.display();
    }
    else if (choice == 3)
    {
        std::string octal;
        getline(std::cin, octal);
        NumberSystemsConverter conversion(8, octal);
        conversion.display();
    }
    else if (choice == 4)
    {
        std::string hex;
        getline(std::cin, hex);
        NumberSystemsConverter conversion(16, hex);
        conversion.display();
    }
}

NumberSystemsConverter.h

#ifndef NUMBERSYSTEMSCONVERTER_H
#define NUMBERSYSTEMSCONVERTER_H

#include <cstdint>
#include <string>

typedef long double d32;
typedef std::uint32_t u32;

class NumberSystemsConverter
{
private:
    d32 decimal;
    std::string binary;
    std::string octal;
    std::string hex;

    int findDecimalPoint(const std::string&) const;
    void decimalToBinary();
    void stringToDecimal(unsigned, const std::string&);
    void binaryToOctal();
    void binaryToHex();

public:
    NumberSystemsConverter(const std::string&); // binary input
    NumberSystemsConverter(d32); // decimal input
    NumberSystemsConverter(unsigned, const std::string&); // octal/hex input
    void display() const;
};

#endif

NumberSystemsConverter.cpp

#include <algorithm>
#include <cmath>
#include <iomanip>
#include <iostream>
#include <sstream>
#include "NumberSystemsConverter.h"

NumberSystemsConverter::NumberSystemsConverter(const std::string &binary)
{
    this->binary = binary;
    stringToDecimal(2, binary);
    binaryToOctal();
    binaryToHex();
}

NumberSystemsConverter::NumberSystemsConverter(d32 decimal)
{
    this->decimal = decimal;
    decimalToBinary();
    binaryToOctal();
    binaryToHex();
}

NumberSystemsConverter::NumberSystemsConverter(unsigned base, const std::string &str)
{
    if (base == 8)
    {
        octal = str;
        stringToDecimal(base, octal);
        decimalToBinary();
        binaryToHex();
    }
    else if (base == 16)
    {
        hex = str;
        stringToDecimal(base, hex);
        decimalToBinary();
        binaryToOctal();
    }
}

int NumberSystemsConverter::findDecimalPoint(const std::string &str) const
{
    int decimalPointIndex = str.find('.');

    if (decimalPointIndex == std::string::npos)
        decimalPointIndex = str.size(); // index identified as end if not found

    return decimalPointIndex;
}

void NumberSystemsConverter::decimalToBinary()
{
    binary = "";
    u32 integerPart = static_cast<std::uint32_t>(decimal);
    d32 decimalPart = decimal - integerPart;

    do
    {
        binary += ((integerPart % 2 == 0) ? '0' : '1');
        integerPart /= 2;
    } while (integerPart > 0);

    std::reverse(binary.begin(), binary.end());

    if (decimalPart > 0.0)
    {
        binary += '.';

        while (decimalPart != 1.0)
        {
            decimalPart *= 2.0;
            binary += ((decimalPart < 1.0) ? '0' : '1');
            if (decimalPart > 1.0) decimalPart -= 1.0;
        }
    }
}

void NumberSystemsConverter::stringToDecimal(unsigned base, const std::string &str)
{
    int decimalPointIndex = findDecimalPoint(str);
    d32 power = (d32)decimalPointIndex - 1;
    decimal = 0.0;

    // loops through entire string (not both sides of decimal point separately)
    for (auto iter = str.cbegin(); iter != str.cend(); ++iter)
    {
        if (*iter != '.')
        {
            char strChar = toupper(*iter);
            d32 charValue = strChar - ((isdigit(strChar)) ? '0' : '7');
            decimal += charValue * (d32)std::pow(base, power);
        }
        else
            power = 0.0;

        power--;
    }
}

void NumberSystemsConverter::binaryToOctal()
{
    std::ostringstream octalSS;
    std::string rightDecimal = "";
    u32 rightValue = 0;

    int decimalPointIndex = findDecimalPoint(binary);
    const std::string::iterator iter = std::find(binary.begin(), binary.end(), '.');

    std::string leftDecimal = binary.substr(0, decimalPointIndex);
    u32 leftValue = std::stoul(leftDecimal, NULL, 2);

    if (iter != binary.cend())
    {
        rightDecimal = binary.substr(decimalPointIndex+1, binary.size());

        while (rightDecimal.size() % 3 != 0)
        {
            rightDecimal += '0';
        }

        rightValue = std::stoul(rightDecimal, NULL, 2);
    }

    octalSS << std::oct << leftValue << '.' << std::oct << rightValue;
    octal = octalSS.str();
}

void NumberSystemsConverter::binaryToHex()
{
    std::ostringstream hexSS;
    std::string rightDecimal = "";
    u32 rightValue = 0;

    int decimalPointIndex = findDecimalPoint(binary);
    const std::string::iterator iter = std::find(binary.begin(), binary.end(), '.');

    std::string leftDecimal = binary.substr(0, decimalPointIndex);
    u32 leftValue = std::stoul(leftDecimal, NULL, 2);

    if (iter != binary.cend())
    {
        rightDecimal = binary.substr(decimalPointIndex+1, binary.size());

        while (rightDecimal.size() % 4 != 0)
        {
            rightDecimal += '0';
        }

        rightValue = std::stoul(rightDecimal, NULL, 2);
    }

    hexSS << std::uppercase << std::hex << leftValue << '.' << std::hex << rightValue;
    hex = hexSS.str();
}

void NumberSystemsConverter::display() const
{
    std::cout << "\n  * Binary : " << std::setprecision(10) << binary;
    std::cout << "\n  * Decimal: " << std::setprecision(10) << decimal;
    std::cout << "\n  * Octal  : " << std::setprecision(10) << octal;
    std::cout << "\n  * Hex    : 0x" << std::setprecision(10) << hex << "\n";
}

Answer 1

Behind all the ifs in main(), I see lurking a parameter:

unsigned inputBases[] = { 2, 10, 8, 16 };
unsigned inputBase = inputBases[choice - 1];

std::string input;
std::getline(std::cin, input);
NumberSystemsConverter conversion(inputBase, input);
conversion.display();

Of course, in order for that to work, you need to make sure that all bases are treated equally in the two-args constructor. A string containing a number in base-10 or base-2 should be usable in NumberSystemsConverter(base, input). Which is just good business anyway; who wants a converter that only selectively knows how to convert? :)

That probably means making sure stringToDecimal(base, str) can handle decimal and binary as well. At which point, your constructor turns into

NumberSystemsConverter::NumberSystemsConverter(unsigned base, const std::string &str)
{
    strToDecimal(base, str);
    decimalToBinary();
    binaryToHex();
    binaryToOctal();
}

By the way, I'm not really liking that those conversion functions set results in the object rather than returning a value.

Answer 2

There is much to review here, although I still don't quite know how to make it vastly better. I'll mention a few things that can be changed (in addition to @cHao's advice).

• The typedefs in the header don't really help with anything, so they can be removed.

• It may help to have separate display functions for each of the number types, rather than just one for all of them. The user may not want to display them all at once, or even in the given way.

• The purpose of the multiple constructors is to allow different conversion functions to be called depending on the input type. It might just be easier to have one constructor set one data member (also based on the input type). If the constructor is still to call these conversion functions, then it can probably call all of them. If not, then the constructor can all neither of them, and the user will have to call them individually as desired.

• findDecimalPoint() looks more suitable as a free function. It can easily take any string, not just a member string, and perform its operations.

• NULL is being used in std::stoul(). Since this is C++11, nullptr would be preferred.

• This for loop:

  for (auto iter = str.cbegin(); iter != str.cend(); ++iter)
  

  can become a range-based for loop:

  for (auto& iter : str)
  

• This can be shortened:

  const std::string::iterator iter = std::find(binary.begin(), binary.end(), '.');
  

  by using auto:

  const auto iter = std::find(binary.begin(), binary.end(), '.');