-1

Hello I have written this simple code to expand an 2d array when no of elements get greater than 5 but it doesn't seem to work.

#include<iostream>
using namespace std;

void expand(char ** &arr, int rows, int noofelements )
{
    char** temparr;
    rows += 5;
    temparr = new char* [rows];
    for (int i = 0; i < noofelements; i++)
    {
        temparr[i] = arr[i];
    }
    delete[]arr;
    arr = temparr;
    cout << "******\narray was expanded\n*****\n";
}

int main()
{
    char** arr;
    int rows = 5,noofelements=0;
    arr = new char* [rows];
    for (int i = 0; i < rows; i++)
    {
        arr[i] = new char[50];
    }
    while (1)
    {
        if (noofelements < rows)
        {
            cout << "enter string\n";
            cin.getline(arr[noofelements++], 50);
        }
        else
        {
            expand(arr, rows, noofelements);
            cout << "enter string\n";
            cin.getline(arr[noofelements++], 50);
        }
    }
}

after the expand function is called program randomly ends and after using the debugger I get an error of exception thrown.

wohlstad
  • 12,661
  • 10
  • 26
  • 39
GA Haroon
  • 55
  • 2
  • 4
    look carefully what memory you allocate in main and compare that to the memory allocations in `expand`. Once you found the mistake start to use `std::vector` please – 463035818_is_not_an_ai Mar 29 '23 at 13:16
  • 2
    A side note: in c++ it is recommended to use `std::vector` instead of raw c arrays. Also see this: [using namespace std](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). – wohlstad Mar 29 '23 at 13:17
  • 1
    The code allocates an array of pointers, then assigns values to the memory that each pointer points at. The pointers haven't been initialized, so the code is writing to random places. Each pointer should be initialized to point at an array of elements of whatever size is appropriate. That means another parameter to the function to tell the code how many columns are needed. – Pete Becker Mar 29 '23 at 13:22
  • 3
    C++ core guidelines : [avoid calling new/delete explicitly](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-newdelete). Why are you still using "C" style arrays? std::vector has "only" been in C++ for 25 years. – Pepijn Kramer Mar 29 '23 at 13:29
  • 2
    Look at the values of `rows` and `noofelements` in `main` before and after the expansion. – molbdnilo Mar 29 '23 at 13:35
  • 1
    *Hello I have written this simple code*-- And that is the issue -- you thought only a "simple code" is what will work. It is more difficult than you expected, which is why `std::vector` exists. – PaulMcKenzie Mar 29 '23 at 13:46

1 Answers1

0

First of all, I understand that you learn CPP and the low level memory allocation.

This should not be done In C++. Basically, the use of raw pointers for owned memory is discouraged.

But, many teachers start with new and delete before using the only suitable containers for this task: std::vector and std::string

So, let us look into your problem.

First, if you expand a 2d-array with 5 elements, then you incremnt "rows" by 5 in your function. But since the passed the "rows" variable by value, a copy will be made, when entering the function. Then the copy will be incremented by 5. If the function returns, the out rows, set in the main function, will still be 5 and not 10.

And if you then try to access elements 5..9, you have an out of bounds problem, with undefined behavior. Many IDEs/Compilers will show a runtime error in debug mode.

Solution: Pass "rows" as reference: void expand(char**& arr, int& rows, int noofelements)


The next problem is in your "expand function. See, in main your are allocating memory for each row pointer. So, you assign arr[i] = new char[50];

This you do not do for the expanded elements in you sub function. You only copy the original 5 pointers. The other 5 new elements undeterminated. The have no meaning full value.

And, if you try later in main to put a value there, then there is no allocated memory. The system will show undefined bahaviour, or, crash.

The solution is, that you, like in main, also alllocate new char[50] elements for all your new, expanded pointers.

This you could do like the below:

#include<iostream>
using namespace std;

void expand(char**& arr, int& rows, int noofelements)
{
    char** temparr;
    rows += 5;
    temparr = new char* [rows];
    int i;
    for (i = 0; i < noofelements; i++)
    {
        temparr[i] = arr[i];
    }
    for (; i < rows; ++i) {
        temparr[i] = new char[50];
    }
    delete[]arr;
    arr = temparr;
    cout << "******\narray was expanded\n*****\n";
}

int main()
{
    char** arr;
    int rows = 5, noofelements = 0;
    arr = new char* [rows];
    for (int i = 0; i < rows; i++)
    {
        arr[i] = new char[50];
    }
    while (1)
    {
        if (noofelements < rows)
        {
            cout << "enter string\n";
            cin.getline(arr[noofelements++], 50);
        }
        else
        {
            expand(arr, rows, noofelements);
            cout << "enter string\n";
            cin.getline(arr[noofelements++], 50);
        }
    }
}

I assume that this is only test code, so, I do not comment on further problems.

A M
  • 14,694
  • 5
  • 19
  • 44