0

For some reason, the first output my program is giving, is garbage value, while the second output is correct.

This is a problem from HackerRank.

I know this question has already been asked by someone else. I just want to know what the problem is in my code.

#include <stdio.h>

int main()
{
    int index,query;
    int count1 = 0;
    scanf("%d%d",&index,&query);
    for(int i=0;i<index;i++)
    {
        int b;
        scanf("%d",&b);
        int A[index][b];
        for(int j=0;j<b;j++)
        {
            scanf("%d",&A[i][j]);
        }
        count1++;
        if(count1<index)
        {
            continue;
        }
        int count2=0;
        while(count2<query)
        {
            int d,e;
            scanf("%d%d",&d,&e);
            printf("%d\n",A[d][e]);
            count2++;
        }
    }
    return 0;
}

If the input is:

2 2
3 1 5 4
5 1 2 8 9 3
0 1
1 3

Then the output should be:

5
9

But instead, my output is:

garbage
9

gsamaras
  • 71,951
  • 46
  • 188
  • 305
Blue Dice
  • 155
  • 1
  • 10
  • 2
    Your code could really benefit from meaningful variable names. – Eugene Sh. Jul 24 '19 at 16:54
  • ah yes. i will update them in a minute. – Blue Dice Jul 24 '19 at 16:56
  • How does that code compile when you have undefined variables like `count` in it? – Chris Turner Jul 24 '19 at 17:00
  • @Aru Viser What the code is supposed to do? – Vlad from Moscow Jul 24 '19 at 17:00
  • 1
    @ChrisTurner OP is renaming their variables... in progress . . – gsamaras Jul 24 '19 at 17:02
  • 1
    @Blobonat, that won't do. – Blue Dice Jul 24 '19 at 17:02
  • ah sorry. i didn't do the edits properly. – Blue Dice Jul 24 '19 at 17:03
  • 1
    Sorry, but your code is not close to a solution to the problem. The input starts with two numbers, the number of arrays *n* and the number of queries *q*. So first you have to read those. (I see you are making edits and appear to be using `index` for *n* and `query` for *q*. You might do better either to stick to the names in the problem or to use more descriptive names like `NumberOfArrays` instead of `index`.) Then the input has *n* lines, each with some number of values. So your program needs to read each of those lines… – Eric Postpischil Jul 24 '19 at 17:05
  • 1
    So your program needs to be able to process lines with varying numbers of values in them. – Eric Postpischil Jul 24 '19 at 17:06
  • the edits are done. sorry for the inconveniences – Blue Dice Jul 24 '19 at 17:06
  • That initial work of reading the values would be inside one loop that iterates *n* times, once for each line. **After** that loop is done, you need a **separate** loop to read and process the queries. Your current program has code to handle the queries inside the main loop. That cannot work. So, you need to redesign your program considerably. To start with, just forget about the queries. Write code to read the values and store them and then, after they are all read, print them out in order. Once that is working, add code to handle the queries. – Eric Postpischil Jul 24 '19 at 17:08
  • Been a while since I've done C, but shouldn't you use malloc to allocate memory for the array if a value cannot be provided before compilation? – ToninGuy3n Jul 24 '19 at 17:10
  • Note that, in order to keep the values you are storing, you must dynamically allocate memory using `malloc`. That should be done once for each line. For each line, after you read its *k*, allocate space for *k* values, then store the values in that space. Additionally, you will need to record those allocates spaces. To do that, after you read the *n* at the beginning of the program, allocate space for *n* pointers to values. Then, as you get space for each line, record the address of the space in that array you allocate at the beginning. – Eric Postpischil Jul 24 '19 at 17:13
  • 2
    @ToninGuy3n I feel empathy for you. [Variable Length Array became legal with C99](https://en.wikipedia.org/wiki/Variable-length_array). – gsamaras Jul 24 '19 at 17:57
  • 1
    @gsamaras Interesting. Just did a quick look up on it, and it's apparently not compatible on certain compilers. Another interesting read on how the [Linux Kernel removes uses of VLAs](https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kills-The-VLA) – ToninGuy3n Jul 25 '19 at 02:32
  • Worth sharing link @ToninGuy3n, but if it's Standard C99, it's Standard . . ;) – gsamaras Jul 25 '19 at 16:14

3 Answers3

4

Disclaimer

I didn't even click the link, so I do not know if the solution is correct, but assuming that you got the logic right..

The problem

is that you populate in stages a local to the body of the for loop 2D array, which at the end of your processing, you expect to have it accessible (I mean the complete matrix, populated from every single iteration of the for loop).

Instead, you get only the last's iteration declared array, that's why you get only the A[1][3] element right, and not the A[0][1], since the 2nd row is populated in the last (2nd iteration), while the 1st row is populated in the first iteration (of the firstly declared A), which goes out of scope as soon as the first iteration terminates.

The fix

So, what you need to fix this is to dynamically allocate memory for your matrix, and every time a new dimension for the columns is inputed, resize it with realloc().

I believe that the explanation I have in 2D dynamic array (C) will help you, since what you want is the number of rows fixed, and the number of columns adjustable on every iteration.

Below is an illustration based on the link I shared above, which visualizes what exactly is your matrix (a 1D array of pointers), and shows how the code below manipulates it:

enter image description here

Full code example:

#include <stdio.h>
#include <stdlib.h>

int main()
{
    int index,query;
    int count1 = 0;
    scanf("%d%d",&index,&query);

    // An array of `index` (e.g. 2) pointers to integers
    int *A[index];
    // Initialize all the pointers to NULL
    for(int k = 0; k < index; ++k)
      A[k] = NULL;

    for(int i=0;i<index;i++)
    {
        int b;
        scanf("%d",&b);

        // Replaced your 'int A[index][b];' with the following:
        // Every time a new number of columns (that's 'b') is read,
        // we need to adjust the numbers of columns our matrix ('A') has.
        // That means, that for every pointer (row), we need to re-allocate
        // the number of columns it points to, which is basically a 1D array, of dimension 'b'
        for(int k = 0; k < index; ++k)
          A[k] = realloc(A[k], b * sizeof(int) );

        for(int j=0;j<b;j++)
        {
            scanf("%d",&A[i][j]);
        }
        count1 ++;
        if(count1<index)
        {
            continue;
        }
        int count2=0;
        while(count2<query)
        {
            int d,e;
            scanf("%d%d",&d,&e);
            printf("%d\n",A[d][e]);
            count2++;
        }
    }

    // Free the dynamically allocated memory
    for(int k = 0; k < index; ++k)
          free(A[k]);
    return 0;
}

Output (for the input provided):

5
9

Pro-tip: The typical methodology of calling realloc() is to use a specific pointer for the reallocation, test that pointer and, if everything worked out ok, change the old pointer, as explained in Does realloc overwrite old contents?, which I didn't do in that post for the sake of being "laconic".

gsamaras
  • 71,951
  • 46
  • 188
  • 305
1

Hint : Variable sized arrays need to be dynamically allocated, here's how to it in C

int rows; 
scanf("%d",&rows);

//2D array 
int**A = malloc(sizeof(int*)*rows); //allocate number of rows

//for each row allocate number of colums 
for(int i = 0; i < rows; i++)
{
   int cols;
   scanf("%d",&cols);
   A[i] = malloc(sizeof(int)*cols);
}
gsamaras
  • 71,951
  • 46
  • 188
  • 305
Midnight Exigent
  • 615
  • 4
  • 15
  • 1
    OP isn't asking for code solution. They're asking for an explanation on what they did wrong. – ToninGuy3n Jul 24 '19 at 17:12
  • I did not give the solution, as what he did wrong was explained by someone else, this is supposed to be a hint on how to solve the problem. – Midnight Exigent Jul 24 '19 at 17:47
  • 1
    Hi @WalidJabari probably by other you mean me! :) I updated my answer with a fix BTW.. I understand your intention for your hint, but notice that [Variable Length Array became legal with C99](https://en.wikipedia.org/wiki/Variable-length_array). – gsamaras Jul 24 '19 at 17:59
  • Hi @gsamaras indeed, I just read your updated answer. Although VLA are legal, some compilers (like MSVC) will not allow something like `int n = 5; int arr[n];` I personally prefer to stay away from VLA – Midnight Exigent Jul 24 '19 at 18:16
1

The C VLA is not suitable here. It seems you need to allocate memory dynamically. The only VLA that can be used is an array of pointers to other arrays. All other arrays should be allocated dynamically.

Something like the following.

#include <stdio.h>
#include <stdlib.h>

int main( void ) 
{
    size_t number_of_arrays;
    size_t number_of_queries;

    scanf( "%zu%zu", &number_of_arrays, &number_of_queries );

    int **a = malloc( number_of_arrays * sizeof( int * ) );

    for ( size_t i = 0; i < number_of_arrays; i++ )
    {
        size_t size;
        scanf( "%zu", &size );

        a[i] = malloc( size * sizeof( int ) );

        for ( size_t j = 0; j < size; j++ ) scanf( "%d", &a[i][j] );
    }

    for ( size_t i = 0; i < number_of_queries; i++ )
    {
        size_t array_index;
        size_t element_index;

        scanf( "%zu%zu", &array_index, &element_index );

        printf( "%d\n", a[array_index][element_index] );
    }

    for ( size_t i = 0; i < number_of_arrays; i++ ) free( a[i] );
    free( a );                                                         
}

If to input

2 2
3 1 5 4
5 1 2 8 9 3
0 1
1 3

then the program output will be

5
9

As for your code then it is invalid. For example the variable b is not initialized so the declaration of the array has undefined behavior.

int b;
scanf("%d",&b);
int A[index][b];
            ^^^
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335