Converting numbers to another base

[bheadmaster]

Hello, I have recently solved a problem from a book:
"Given a string that represents a base X number, convert it to equivalent string in base Y,
2 ≤ X, Y ≤ 36. For example: “FF” in base X = 16 (Hexadecimal) is “255” in base Y1 = 10
(Decimal) and “11111111” in base Y2 = 2 (binary)."

I have solved it but I feel like my code is too large and ugly. Was there a more elegant way to solve this?

Here's my code:

#include <iostream>
#include <string>

using namespace std;



int main()
{
	int X, Y, value = 0;
	string strx, stry = "";
	cin >> X >> strx >> Y;

	while(strx != "")
	{
		value = (X * value) + ( isalpha(strx[0]) ? (strx[0] - 'A' + 10) : (strx[0] - '0') );
		strx = strx.substr(1, strx.length() - 1);
	}

	while(value)
	{
		stry = static_cast<char>( ((value % Y) < 9) ? ((value % Y) + '0') : ((value % Y) - 10 + 'A') ) + stry;
		value /= Y;
	}
	
	cout << stry << endl;
	
	return 0;
}

[dhayden]

First, there's a bug at line 22. value % Y < 9 should be value % Y < 10.

The code could be more efficient since each time through both loops, you copy a portion of one of the strings. The first loop is easy to fix, just walk through the characters in the string:

for (int i = 0; i < strx.size(); ++i) {
	value = (X * value) + ( isalpha(strx[i]) ? (strx[i] - 'A' + 10) : (strx[i] - '0') );
}

Or better yet:

for (auto ch : strx) {
	value = (X * value) + ( isalpha(ch) ? (ch - 'A' + 10) : (ch - '0') );
}

As for the second loop, the problem is that you're adding the new digits to the beginning, which means copying all the data in the string. There are two ways to solve this. You could add the new digits to the end of the string, and then reverse the string at the end, or you could create an empty string that's guaranteed to be large enough to hold the data, fill it in from back to front, and then take the appropriate substring at the end.

Nice job by the way. Lots of beginners struggle just to convert from one fixed base to another. This is a harder problem and you have the logic correct (other than that one bug). Just as important, you recognized that the problem probably had a simpler solution.

[andywestken]

I would probably factor out the char -> value / value -> char conversions into functions...

This also helps make the code more "self documenting".
https://en.wikipedia.org/wiki/Self-documenting

Terser is not necessarily better. Clarity is important, too. And these functions are tiny enough that they will be inlined so there is no performance penalty.

And the function-based version is prob a better starting point if you wanted to add error handling.

Andy

#include <iostream>
#include <string>

using namespace std;

inline int digitToValue(char digit)
{
    return isalpha(digit) ? (digit - 'A' + 10) : (digit - '0');
}

inline char valueToDigit(int value)
{
    return static_cast<char>( (value < 10) ? (value + '0') : (value - 10 + 'A') );
}

int main()
{
    int X = 0, Y = 0; // I buy into init all variables (unless good reason)
    string strx; // no need for = "" as that's what default constructor does
    cin >> X >> strx >> Y;

    int value = 0; // not needed till here

    while( !strx.empty() ) // rather than != ""
    {
        value = (X * value) + digitToValue(strx[0]);
        strx.erase(0, 1); // a bit better than strx = strx.substr(1, strx.length() - 1);
    }

    string stry; 

    while(value)
    {
        stry = valueToDigit(value % Y) + stry;
        value /= Y;
    }
	
    cout << stry << endl;

    return 0;
}

or with dhayden's suggested changes (pre C++11 friendly take...)

#include <iostream>
#include <string>
#include <algorithm> // for std::reverse()

using namespace std;

inline int digitToValue(char digit)
{
    return isalpha(digit) ? (digit - 'A' + 10) : (digit - '0');
}

inline char valueToDigit(int value)
{
    return static_cast<char>( (value < 10) ? (value + '0') : (value - 10 + 'A') );
}

int main()
{
    int X = 0, Y = 0; // I buy into init all variables (unless good reason)
    string strx; // no need for = "" as that's what default constructor does
    cin >> X >> strx >> Y;

    int value = 0; // not needed till here

    const int size = strx.size(); // don't have to get size each time
    for (int i = 0; i < size; ++i)
    {
        value = (X * value) + digitToValue(strx[i]);
    }

    string stry; 

    while(value)
    {
        stry += valueToDigit(value % Y);
        value /= Y;
    }

    reverse(stry.begin(), stry.end()); // reverse with standard algorithm
	
    cout << stry << endl;
	
    return 0;
}