Array Dynamic resize in heap

Question

I have answered a Question in Stackoverflow link.

a) Create a function called resize that can be used to increase the size of integer arrays dynamically. The function takes three parameters. The first parameter is the original array, the second parameter is the size of this array, and the third parameter is the size of the larger array to be created by this function. Make sure that you allocate memory from the heap inside this function. After allocating memory for the second array the function must copy the elements from the first array into the larger array. Finally, the function must return a pointer to the new array.

b. In main, allocate an array on the heap that is just large enough to store the integers 5, 7, 3, and 1.

c. Resize the array to store 10 integers by calling the resize function created in step a. Remove the old (smaller) array from the heap. Add the numbers 4, 2, and 8 to the end of the new array.

d. Write a sort function that sorts any integer array in increasing order.

e. Use the sort function to sort the array of numbers in c above. Display the sorted numbers.

Is there a Dangling pointer issue.

#include <array>
#include <iostream>

void swap(int *xp, int *yp) 
{ 
    int temp = *xp; 
    *xp = *yp; 
    *yp = temp; 
} 

//Bubble Sort
bool sort(int arr[], int size)
{
    for( int i = 0; i< size -1; i++)
    {
        for( int j = 0; j < size - i -1; j++)
        {
            //descending order
            if(arr[j]<arr[j+1])
            {
                swap(&arr[j], &arr[j+1]);
            }
        }
    }
    return true;
}

void Print(int Array[], int nSize)
{
    for( int i = 0; i < nSize; i++)
    {
        std::cout<<" "<<Array[i];
    }
    std::cout<<"\n";
}

void Resize( int *&Array, const int& nSizeOld, const int& nSize )
{
    int * newArray = new int[nSize];

    //Copy Elements of the Array
    for(int i = 0; i< nSize; i++)
    {
        newArray[i] = Array[i];
    }

    delete[] Array;

    //Assign ptr of Prev to new Array
    Array =  newArray;
}

int _tmain(int argc, _TCHAR* argv[])
{
    const int kNewSize = 10, kSize = 5;
    int *pMyArray = new int[kSize];

    //Set Values
    for( int i = 0; i< kSize; ++i ) 
    {
        pMyArray[i] = i * 5;
    }

    Resize( pMyArray, kSize, kNewSize );

    //Set Values
    for( int i = kSize; i< kNewSize; ++i ) 
    {
        pMyArray[i] = i * 10;
    }

    Print(pMyArray, kNewSize);

    sort(pMyArray, kNewSize);

    Print(pMyArray, kNewSize);

    if( pMyArray!=NULL )
    {
        delete[] pMyArray;
    }

    return 0;
}

Answer 1

If you had tagged this code as C, it would have been acceptable. Since you tagged it as C++, it's horrible.

Instead of writing your own swap function, there's already std::swap in <algorithm>.

Instead of writing bubble sort yourself, just use std::sort, also from <algorithm>.

Instead of using arrays and resizing them yourself, just use std::vector<int>, from <vector>.

After applying these transformations, you cannot have a dangling pointer anymore since your code is completely pointer-free.

As part of an exercise for learning the basic operations on memory management, it's ok to write code like this, but never ever use such code in production. In production the code should look like this:

#include <algorithm>
#include <iostream>
#include <vector>

void Print(const std::vector<int> &nums)
{
    for(int num : nums)
    {
        std::cout << " " << num;
    }
    std::cout << "\n";
}

int main()
{
    std::vector<int> nums { 5, 7, 3, 1 };

    // There's probably a more elegant way to add the elements to the vector.
    nums.push_back(4);
    nums.push_back(2);
    nums.push_back(8);

    std::sort(nums.begin(), nums.end());

    Print(nums);
}

By the way, your original code doesn't have any dangling pointer as well. Well done.

You don't need the != NULL check before the delete[] since that pointer cannot be null. In modern C++ (since C++11 I think) you would also write nullptr instead of NULL. The reason is that historically NULL had not been guaranteed to be of pointer type.

Have a look at https://en.cppreference.com/w/cpp/algorithm for more algorithms that you shouldn't implement yourself in C++.

I would have liked to write the push_back block in a shorter way, as well as the Print function. I'm sure there's a more elegant way, I just don't know it.

Answer 2

The code is obviously wrong: your compiler should have warmed you that Resize() never uses its nSizeOld parameter.

Answer 3

1. Your code is too low-level. It expresses implementation details instead of intent. That's why your code looks like "C with couts instead of printf and new/delete instead of malloc/free" instead of C++.

2. Roland Illig has already told you that you should use std::swap instead of building a new one from scratch. You should use existing libraries, especially the standard library, whenever possible.

   That said, your own implementation of swap is also questionable. This is C++, not C. We have references. Using pointers makes the code less readable, and puts burden on the user of the function. So you should change it to:

   void swap(int& x, int& y)
   {
       int temp = x;
       x = y;
       y = temp;
   }
   

   And the calls to it can be changed from swap(&foo, &bar) to swap(foo, bar). Still, std::swap is preferable.

3. Again, Roland Illig has already told you that you should use the std::sort instead of building a new bubble sort from scratch. std::sort typically uses quicksort, which has \$O(n \log n)\$ time complexity; whereas bubble sort has \$O(n^2)\$ time complexity. It should be obvious that std::sort is much more efficient.

4. Your parameter lists are so C-ish. (pointer, size) parameter pairs are everywhere. They are error-prone. Consider using spans. (Spans are currently not available in the standard library; consider using the one from GSL)

   You even have parameter lists like (int*& Array, const int& nSizeOld, const int& nSize). Don't pass by const reference for builtin types. Just pass by value, as in int nSizeOld, int nSize. And letting a pointer denote an array with sizes littered everywhere holds a great welcome party for errors.

5. Don't use _tmain and _TCHAR. They are not portable. (Strictly speaking, they are not proper C++.) You should write in ISO standard C++. Use main and char instead.

   // Correct prototype of the main function
   int main(int argc, char* argv[])
   {
       // ...
   }
   

6. Don't make such liberal use of "naked" news and deletes. Explicit calls to news and deletes are very error prone. std::vectors should be preferred from the beginning.

7. You have four for loops in total. The first three use i++, whereas the last one uses ++i. Please consistently use ++i.

As a conclusion: you should refactor your code to express intent.