Simplifying my program to convert numbers to from one base to another

Question

I'm taking a beginner C++ course. I received an assignment telling me to write a program that converts an arbitrary number from any base between binary and hex to another base between binary and hex. I was asked to use separate functions to convert to and from base 10. It was to help us get used to using arrays. (We already covered passing by reference previously in class.) I already turned this in, but I'm pretty sure this wasn't how I was meant to do it:

#include <iostream>
#include <conio.h>
#include <cstring>
#include <cmath>

using std::cout;
using std::cin;
using std::endl;

int to_dec(char value[], int starting_base);
char* from_dec(int value, int ending_base);

int main() {
    char value[30];
    int starting_base;
    int ending_base;
    cout << "This program converts from one base to another, so long as the bases are" << endl
        << "between 2 and 16." << endl
        << endl;
input_numbers:
    cout << "Enter the number, then starting base, then ending base:" << endl;
    cin >> value >> starting_base >> ending_base;
    if (starting_base < 2 || starting_base > 16 || ending_base < 2 || ending_base > 16) {
        cout << "Invalid base(s). ";
        goto input_numbers;
    }
    for (int i=0; value[i]; i++) value[i] = toupper(value[i]);
    cout << "Base " << ending_base << ": " << from_dec(to_dec(value, starting_base), ending_base) << endl
        << "Press any key to exit.";
    getch();
    return 0;
}

int to_dec(char value[], int starting_base) {
    char hex[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
    long int return_value = 0;
    unsigned short int digit = 0;
    for (short int pos = strlen(value)-1; pos > -1; pos--) {
        for (int i=0; i<starting_base; i++) {
            if (hex[i] == value[pos]) {
                return_value+=i*pow((float)starting_base, digit++);
                break;
            }
        }
    }
    return return_value;
}

char* from_dec(int value, int ending_base) {
    char hex[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
    char *return_value = (char *)malloc(30);
    unsigned short int digit = (int)ceil(log10((double)(value+1))/log10((double)ending_base));
    return_value[digit] = 0;
    for (; value != 0; value/=ending_base) return_value[--digit] = hex[value%ending_base];
    return return_value;
}

I'm pretty sure this is more advanced than it was meant to be. How do you think I was supposed to do it?

I'm essentially looking for two kinds of answers:

1. Examples of what a simple solution like the one my teacher probably expected would be.
2. Suggestions on how to improve the code.

Answer 1

I don't think you need the inner loop:

for (int i=0; i<starting_base; i++) {

What is its purpose?

Rather, you should get the character at value[ pos ] and convert it to an integer. The conversion depends on base, so it may be better to do it in a separate function.

You are defining char hex[ 16 ] twice, once in each function. It may better to do it at only one place.

---

EDIT 1: Since this is "homework" tagged, I cannot give you the full answer. However, here is an example of how to_dec() is supposed to work. (Ideally, you should have constructed this!)

Input:

  char * value = 3012, 
  int base = 4, 

Math:

Number = 3 * 4^3 + 0 * 4^2 + 1 * 4^1 + 2 * 4^0 = 192 + 0 + 4 + 2 = 198

Expected working of the loop:

  x = 0
  x = 4x + 3 = 3
  x = 4x + 0 = 12
  x = 4x + 1 = 49
  x = 4x + 2 = 198

  return x;

---

EDIT 2:

Fair enough! So, here is some more :-)

Here is a code sketch. Not compiled or tested though. This is direct translation of the example I provided earlier.

unsigned
to_dec( char * inputString, unsigned base )
{
  unsigned rv = 0; // return value
  unsigned c;      // character converted to integer

  for( char * p = inputString; *p; ++p ) // p iterates through the string
  {
    c = *p - hex[0];
    rv = base * rv + c;
  }

  return rv;
}

Answer 2

I would stay away from GOTO statements unless they are absolutely necessary. GOTO statements are easy to use but will lead to 'spaghetti code'.

Try using a loop instead. Something along the lines of this:

bool base_is_invalid = true;
while ( base_is_invalid ) {

    cout << "Enter the number, then starting base, then ending base:" << endl;
    cin >> value >> starting_base >> ending_base;

    if (starting_base < 2 || starting_base > 16 || ending_base < 2 || ending_base > 16)
        cout << "Invalid number. ";
    else
        base_is_invalid = false;

}

Answer 3

You can initialize arrays by string literals (notice that the terminating \0 is not included because the size of the array doesn't permit that):

char const hex[16] = "0123456789ABCDEF";

Or just use a pointer to the string literal for the same effect:

char const* hex = "0123456789ABCDEF";