1

I'm writing a C++ program that converts a decimal number to binary and hexadecimal. The problem is that for some reason it concatenates the number "1875954912" to both representations every time.

I've tried a bunch of things - mainly changing up how the program calculates numArrayLength and the for-loop in my decToBase function, but I haven't been able to figure out why this happens yet.

The program is not complete by the way - it doesn't turn integers bigger than 9 into letters for the hex representation yet, but that's not my main concern right now.

Here is my code:

#include <iostream>
#include <cmath>
using namespace std;

int howManyBitsNeeded(int someNum, int base) {
    int numOfDivisions = 0;
    while (someNum != 0) {
        someNum = floor(someNum / base);
        numOfDivisions += 1;
    }
    return numOfDivisions;
}


int decToBase(int someNum, int base) {
    int bitsNeeded = howManyBitsNeeded(someNum,base);
    int numArrayLength = bitsNeeded;
    int numArray[bitsNeeded];

    while (bitsNeeded > 0) {
        numArray[bitsNeeded] = (someNum % base);
        someNum = floor(someNum / base);
        bitsNeeded -= 1;
    }

    for (int k = (numArrayLength-1); k >= 0; --k) {
        cout << numArray[(numArrayLength - k)];
    }

}


int main() {
    int inpNum;

    cout << "Enter your number: ";
    cin >> inpNum;

    cout << "Binary representation: " << decToBase(inpNum,2) << endl;
    cout << "Hexadecimal representation: " << decToBase(inpNum,16);

    return 0;
}

And here's what the output looks like:

Enter your number: 25
Binary representation: 110011875954912
Hexadecimal representation: 191875954912

Any help would be greatly appreciated!

Blaze
  • 16,736
  • 2
  • 25
  • 44
dilara
  • 13
  • 2
  • 2
    What does `decToBase` *return*? And *is* it supposed to return something, or only to print something, or do both? – Some programmer dude Oct 25 '19 at 08:07
  • Array indexes start at `0` (not `1`), and end at `size-1` (not `size`) - where `size` is the amount of items in the array. Keep that in mind when reading the `decToBase` function, and specifically the first time `numArray[bitsNeeded] = (someNum % base);` is executed, as well as the output loop. – Sander De Dycker Oct 25 '19 at 08:12
  • It seems you access not allocated index of array, `numArray`. Check while loop in `decToBase`. – Zem Oct 25 '19 at 08:12
  • Firstly, `int numArray[bitsNeeded]` is not standard C++ when `bitsNeeded` is a variable. Use a standard container , like `std::vector` instead. Second, you are treating array indexing as if it is one-based (i.e. elements `1` to `bitsNeeded`). Array indexing in C++ is zero based - valid indices start at `0`. Accessing past the end as you are gives undefined behaviour. – Peter Oct 25 '19 at 08:37

1 Answers1

2

Your decToBase is declared as returning an int, but it doesn't actually return anything. Your compiler should warn you about this. Since you're not returning anything here, change its return type to void. Then instead of trying to print its return value, simply call the function without printing it:

std::cout << "Binary representation: ";
decToBase(inpNum, 2); // this already prints the number, no need to pass it to std::cout
std::cout << endl;

std::cout << "Hexadecimal representation: ";
decToBase(inpNum, 16);
std::cout << std::endl;

Or of course you can change the function to return the string that you want to print instead of printing it inside the function.

Also, there's an issue here:

int numArray[bitsNeeded];

This is out of range when you try to access it here:

while (bitsNeeded > 0) {
        numArray[bitsNeeded] = (someNum % base);

And also later when you try to print it. To get rid of this off by one error, you have to change this to

numArray[bitsNeeded-1] = (someNum % base);

And in the output change it to

cout << numArray[(numArrayLength - k -1)];

And while you're at it, instead of having it as a VLA (which isn't part of C++ and only works if the compiler tolerates it), I would recommend a vector:

std::vector<int> numArray(bitsNeeded+1); // include <vector> for this to work

Furthermore, note that integer division is already truncated, so unless you plan to support negative numbers later on, you can silence a warning about implicit double to int conversion by changing this:

someNum = floor(someNum / base);

To this:

someNum /= base;
Blaze
  • 16,736
  • 2
  • 25
  • 44