0

I am trying to make a 2D array with each sub array of variable length using the following code. When I try to access the values in the array sometimes it gives the correct value for one query but mostly outputs garbage values. I can't seem to understand what I am doing wrong.

#include <iostream>
using namespace std;


int main() {
    
    int size, queries;
    
    cin >> size >> queries;
    
    int **p = new int *[size];
    
    int sub_size;
    
    for (int i =0; i< size; i++) {
        cin >> sub_size;
        int *sub_arr = new int[sub_size]
        
        for (int j=0; j<sub_size; j++) {
            cin >> sub_arr[j];
        }
        
        //p[i] = sub_arr;
        *p = sub_arr;
        p++;
        
    }
    
    int r, c;
    
    
    for (int i=0; i<queries; i++) {
        cin >> r >> c;
        cout << p[r][c] << endl;
    }
    
    delete []p;
     
    return 0;
}

I have made changes to the code now sub array is also declared using new[]

Zohaib Hamdule
  • 540
  • 9
  • 23
  • 4
    Note that `int sub_arr[sub_size];` is a VLA, which is not standard C++. [Why aren't variable-length arrays part of the C++ standard?](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard) – kotatsuyaki Dec 11 '22 at 06:12
  • 1
    From your learning materials, are you familiar with the concept of scope and lifetime? So then how long does `sub_arr` live? So if you set a pointer to point to it and it goes out of scope, what happens? – TheUndeadFish Dec 11 '22 at 06:17
  • Why not simply `std::vector>`? – PaulMcKenzie Dec 11 '22 at 06:17
  • @PaulMcKenzie I am not familiar with std::vector – Zohaib Hamdule Dec 11 '22 at 06:18
  • 4
    @ZohaibHamdule --`std::vector` has been part of C++ for 24 years now. There is no reason to write code in the way you've written it (using `new[]`). – PaulMcKenzie Dec 11 '22 at 06:19
  • @TheUndeadFish how do I prevent sub_arr from going out of scope. Should it stay in the memory until I use delete? – Zohaib Hamdule Dec 11 '22 at 06:25
  • `int sub_arr[sub_size];` is an Automatic allocation. The name comes from the the lifetime being automatically managed for you. At the end of the current block, it's gone. You need to use a dynamic allocation so that you can manage the lifespan yourself or hand off the management to a class that can handle the management for you like a `std::vector` or a smart pointer. If `vector`'s off the table, odds are good that smart pointers are, too. That leaves you with `new` (and a few options inherited from C that are generally harder to use than `new`). – user4581301 Dec 11 '22 at 06:41
  • @ZohaibHamdule [See this](https://godbolt.org/z/n341ccx1o). – PaulMcKenzie Dec 11 '22 at 06:45
  • General rule of thumb: That which you `new` you must `delete`. If you `new[]` it, you `delete[]` it. if you `malloc` `calloc` or `realloc` (and I think I may be missing one here), you must `free` it. If you call a function and get a resource back, check the provider's documentation. It'll either be self-managing somehow or there will be a release function of some sort you need to call. – user4581301 Dec 11 '22 at 06:47

1 Answers1

0

I fixed the code. The problem was that I was incrementing p using p++ so p was no longer pointing to the start of the pointer array. So I used a new pointer called temp to manipulate the 2D array:

int main() {
    
    int size, queries;
    
    cin >> size >> queries;
    
    int **p = new int *[size];
    int **temp = p; //temp also points to the same array of pointers
    
    int sub_size;
    
    for (int i =0; i< size; i++) {
        cin >> sub_size;
        int *sub_arr = new int[sub_size];
        
        for (int j=0; j<sub_size; j++) {
            cin >> sub_arr[j];
        }
        
        //p[i] = sub_arr;
        *temp = sub_arr;
        temp++;
        
    }
    
    int r, c;
    
    
    for (int i=0; i<queries; i++) {
        cin >> r >> c;
        cout << p[r][c] << endl;
    }
    
    delete []p;
     
    return 0;
}
Zohaib Hamdule
  • 540
  • 9
  • 23