I wrote this code to convert from number in denary to a different base
I was curious if anyone has any tips on a better way to write this code
I'm always looking to improve my coding and I find others critiques to be a great way of improving.

In the below code I omit include's and presume the base isn't going to be above a certain number

string convertedNumber(long numberToConvert, int base)
{
    char letterArray[] = {'A','B','C','D','E','F','G','H','I','J','K','L','M',
                          'N','O','P','Q','R','S','T','U','V','W','X','Y','Z'};
    vector<short>baseNum;

    while(numberToConvert != 0)
    {
        baseNum.push_back(numberToConvert % base);
        numberToConvert = numberToConvert / base;
    }

    string convertedNum = "";

    for(int i = baseNum.size()-1 ; i>=0; i--)
    {
        //This is presuming that the base is lower than 36
        if(baseNum.at(i) > 9)
        {
            convertedNum += letterArray[baseNum.at(i)-10];           
        }
        else
        {           
            convertedNum += ((char)(baseNum.at(i) + '0'));
        }
    }   
    return convertedNum;
}

I didn't test the below code, but there is no need for the vector in your code. Just
go directly to string.

//supports only positive numbers
string convertToBase(long num, int radix){
 assert( radix > 0 && radix < 36); //base range[1,36)
 assert( num > 0);
 string ans = "";
 string values = "123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
 while(abs(num)){
   ans.push_back(values[num % radix]);
   num /= radix;
 }
 return ans;
}

Edited 6 Years Ago by firstPerson: n/a

Ah it's a quite interesting little function you have there. Unfortunately, I can't come up with any significant strategy changes to improve your conversion; I would probably have solved this problem in a similar fashion myself.

However, If you really want to keep your code on edge, there are always some optimization techniques you could keep in mind. Like using prefix decrement ( --var ) instead of postfix decrement ( var-- ). I found this site where you can find out why, along with some other useful tips.

Also I just HAD to run some test cases on your code, and results were as I suspected. First of all you might want to add some sort of range checking on your base argument, so that only inputs between 0 and 36 are allowed. In this case you will also have to check for 1, since a base of 1 never allow you to leave the initial while loop. This is a matter of discussion however, whether to allow a base of 1 or not. I think it's possible to represent a number with a base of 1, it would look like this:

Base 10 | Base 1
--------+--------
   1    |   1
   2    |   11
   3    |  111

And so on..

I hope you find my reply useful!

Regards,
Emil Olofsson

Just realized that my function is wrong. It needs an offset of 1, and it needs to be printed backwards. But the general idea stands.

First off, you need to realize that you aren't converting a denary string to a string in another base; you are converting a binary integer into a string of numerals in the given base. This is important in understanding the rest of this.

The traditional approach, developed at MIT sometime in the early 1960s, is a rather simple recursive algorithm. For bases up to 10, it would be:

#include <string>
#include <iostream>
#include <sstream>
#include <exception>

class baseoverflowexception: public std::exception
{
  virtual const char* what() const throw()
  {
    return "My exception happened";
  }
} BaseOverflowException;


std::string bin2base(int value, int base) throw (baseoverflowexception)
{
    std::stringstream out;

    if (base > 10)
        throw (BaseOverflowException);
    else
    {
        if (value < base)
        {
            out << value;
            return out.str();
        }
        else
        {
            out << value % base;
            return bin2base(value / base, base) + out.str();
        }
    }
}

Allowing for an arbitrary base up to 36 complicates this a little, but not tremendously so. Solving this is left as an exercise. :P

Edited 6 Years Ago by Schol-R-LEA: n/a

This article has been dead for over six months. Start a new discussion instead.