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.

• Jamal: Yes, it should. I've tested that function with other bases and received correct results. I'll give this a try. Will this change require other changes in the class, or is this all that's needed?
• cHao: @Jamal: The conversions look more complicated than i've typically seen with base conversion. The fact that you're working with floating-point probably would complicate things a bit, though...so i'd have to look more before i could say whether they could be simplified or merged.
• Jamal: Regarding the last comment in your answer, do you mean that the constructor above should do all the data member assignments (after having the functions return values)?
• cHao: @Jamal: Considering the class's public functions never return a value, the class could actually get by with not having any data members at all other than, say, a to-decimal conversion of the input. All of the other values could easily be found in display(), without saving them at all, if the functions returned their results rather than stuffing them in the object's data fields.
• Jamal: Okay, I've changed each function to return a value. If I were to call display() without having those other data members, would they need to become arguments for display()?

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(), '.');