c++ dynamic memory allocation for arrays

Question

I feel like I'm missing something obvious and I can't figure it out. Basically, it seems the information is correctly being stored in the first for loop. But when I go to print it out in the second for loop its only garbage values. What am I missing? I'm relatively new to this

bignum::bignum(const string &digits)
{
    int length = digits.length();
    ndigits = 0;    

    for (int i = 0; i < length; i++)
    {
        if(isdigit(digits[i]))
        {
            ndigits++;
            digit = new int[ndigits];
            int tmpInt = digits[i] - '0';
            digit[i] = tmpInt;
        }
        if(isalpha(digits[i]))
        {
            break;
        }
        cout <<"step "<< i << " " << digit[i] << endl;
    }

    for (int i = 0; i < ndigits; i++)
    {
        cout << digit[i] << " ";
    }
    cout << endl;
    cout << "digits" << ndigits << endl;
    cout << endl;
}

Answer 1

You are dynamically allocating digit each time the isdigit(digits[i]) condition is true. So, whenever you find a digit, you allocate memory for digit again and the information about previous digits is lost. Further, there is memory leak.

You should allocate digit once before the start of the for loop and count the number of digits you have found and store the found digits in digit.

Also, you should deallocate any piece of dynamically allocated memory whenever you are done with it, so it can be available for reuse.

Answer 2

Assuming that you do not want to use collections because you are completing a learning exercise, here is what you should keep in mind: when you create an array dynamically, it's best to find out how many elements it is going to have upfront, then allocate it once, fill it with data, use it, and de-allocate the result.

For that to work you need to separate your first loop into two loops - one that counts the digits, and another one that fills in the array with the digits that you find. The allocation should happen after the first loop and before the second loop:

ndigits = 0;    
// First, you need to count the digits
for (int i = 0; i < length; i++)
{
    if(isdigit(digits[i]))
    {
        ndigits++;
    }
    if(isalpha(digits[i]))
    {
        break;
    }
}
// Next, allocate the array for your digits
int *digit = new int[ndigits];
int *dp = digit;
// Now go through the string again, and add digits to the newly allocated array
for (int i = 0; i < length; i++)
{
    if(isdigit(digits[i]))
    {
        (*dp++) = (digits[i] - '0');
    }
    if(isalpha(digits[i]))
    {
        break;
    }
}
... // Use your digit[] array here...
// Once you are done, free the array
delete[] digit;

Answer 3

Try this if you want the array to be resized as you get more numbers. This version has room for 10 initially, and increses size by 10 when number 11 comes and so on. A new array is created, and old data is copied to it. Old digit array is deleted, and its pointer is then replaced with the new version.

This method might be slow in some cases (because of the copying to the new bigger when the old is full), but it should be OK if you normally have less than 10 digits. If You normally have more than 10, use a bigger initial number.

int digits_max = 10;
int digits_pos = 0;
int *digit = new int[digits_max];

...

if(isdigit(digits[i])) {
    if (digits_pos == digits_max) {
        int new_max = digits_max + 10;
        int *digit_tmp = new int[new_max];

        // Copy from old array to new
        for (int i = 0; i < digits_max; i++) {
            digit_tmp[i] = digit[i];
        }

        delete[] digit; // Free old memory, or your RAM gets full

        digit = digit_tmp;
        digits_max = new_max;
    }
}
else { // Always break if not int
    break;
}

int tmpInt = digits[i] - '0';
digit[digits_pos] = tmpInt;
digits_pos++;

Before exiting (or when you are finished using the digit-array), you should delete it:

delete[] digit;

Edit: Moved digits_pos++ to end of loop, last version was wrong.