-2

I was solving hackerrank problem https://www.hackerrank.com/challenges/icecream-parlor/problem. If I declare freq[] first, all test cases passed, but if I declare arr[] first, two test cases failed. Can anyone explain why?

#include <bits/stdc++.h>
using namespace std;
int freq[10004]; //This passes
int arr[10004];  //for all test cases
//int arr[10004]; but this fails
//int freq[10004]; for two test cases...why??

int main()
{
    int t;
    cin >> t;
    for (int i = 0; i < t; i++)
    {
        memset(freq, 0, sizeof(freq));
        int m, n, ansi, val;
        cin >> m >> n;
        for (int j = 0; j < n; j++)
        {
            cin >> arr[j];
            freq[arr[j]]++;
        }
        for (int j = 0; j < n; j++)
        {
            freq[arr[j]]--;
            if (freq[m - arr[j]] > 0)
            {

                ansi = j;
                val = m - arr[j];
                cout << j + 1;
                break;
            }
            freq[arr[j]]++;
        }
        for (int j = ansi + 1; j < n; j++)
        {
            if (val == arr[j])
            {
                cout << " " << j + 1 << endl;
                break;
            }
        }
    }
    return 0;
}
CuriouslyRecurringThoughts
  • 2,526
  • 2
  • 9
  • 22
Vishal Kumar
  • 13
  • 3
  • 3
    Its most likely that one of your arrays (probably `arr`) is going out of bounds. I can't really see where it is going out of bounds - but there are lots of places it could be that don't have checks. I would suggest to put bound check just before each access. For example before doing `cin >> arr[j];` put `if ((j > 0) && (j < sizeof(arr)) { do stuff } else { print error}` (pseudo code). Then you can at least see where this is occurring... – code_fodder Jul 04 '19 at 19:38
  • `freq[m - arr[j]]` are you certain that `m - arr[j]` is `>=0` always? If not you have undefined behaviour and swapping two declarations can turn something that appears to work into something that does not appear to work, in any case it would need a fix – 463035818_is_not_a_number Jul 04 '19 at 19:42
  • But why it works when order of declaration is changed? – Vishal Kumar Jul 04 '19 at 19:46
  • @VishalKumar probably because (for one example) arr gets written to out of bounds (i.e. beyond its size) and possibly writes over freq. Or the other way around (freq gets written to below 0 (e.g. `freq[-1]` and overwrites arr... When you switch them freq is no longer after arr so writing to arr beyond the end does not over write freq.... but its overwriting something! - its undefined behaviour meaning - its anyone's guess what happens! – code_fodder Jul 04 '19 at 19:49
  • @VishalKumar Your program has undefined behaviour, that means what it says. A program with a bug can still work. Then some trivial change which shouldn't change anything means it stops working. There's nothing in C++ that says bugs **must** cause a program to fail. – john Jul 04 '19 at 19:53
  • replace `int freq[10004];` with `std::array freq;` and do the same with `arr`. Then replace all uses of the `[]` operator with calls to `std::array::at`. This will make the code run slower, but the program with throw an exception when an out-of-bounds access is made. A debugger usually halts and allows you to take control of the program when an uncaught exception occurs so you can inspect the backtrace to see what and where things when wrong. – user4581301 Jul 04 '19 at 20:00
  • By the way, watch out for `#include ` ([why](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h)) and `using namespace std;` ([why](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)) as both can cause problems. Putting the two together makes the potential for problems much, much worse. When you have a mystery bug, these are two of the first things that should be removed from the code to eliminate them as possible sources of error. – user4581301 Jul 04 '19 at 20:06
  • Thanks a lot to all of you for helping. I just learned something new. My code now works . I added a condition in if statement if(freq[m-arr[j]]>0 && m>arr[j]) – Vishal Kumar Jul 04 '19 at 20:09
  • If you can explain HOW that solves the problem, you should post an answer to the question. No point leaving it dangling. – user4581301 Jul 04 '19 at 20:11
  • the reason it "worked" is probably because accessing `arr` with a negative index you end up in the upper end of `freq` (important: there is no rule that would explain this, thats just what the compiler does to your code because you are doing something wrong), while in the other case a negative index into `arr` ends up in memory not owned by your program – 463035818_is_not_a_number Jul 04 '19 at 20:29
  • I am not able to ask more questions . It says to edit this question according to the complaints. What should I do now? – Vishal Kumar Jul 05 '19 at 15:39

1 Answers1

1

Following on from the comments. Here is a simple example:

// Two arrays of size 10
int arr[10];
int freq[10];

// Write to freq filling it with 1's
for (int i = 0; i < 10; i++);
{
    freq[i] = 1;
}

// Write to arr filling it with 2'.... 
// but by mistake go wrote 11 times when the arr is only 10 long.
for (int i = 0; i < 11; i++);
{
    arr[i] = 2;
}

Where did arr[10] = 2 get written to?

It could be that freq is next in memory and so the first element of freq now contains a 1.

Or it could be that somthing else is after arr - we don't know, its up to the compiler/linker/os as to what that memory contains. This is why its called undefined behaviour - its out of your control.

E.g. after the first loop:

         |arr----------------|freq---------------
Memory:   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 
Written:                      1 1 1 1 1 1 1 1 1 1

Then after the second loop:

         |arr----------------|freq---------------
Memory:   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 
Written:  2 2 2 2 2 2 2 2 2 2 2 1 1 1 1 1 1 1 1 1
                              ^
                        Over written the array

You could catch your issue with some bound checks:

for (int i = 0; i < 11; i++);
{
    if (i >= 0 && i < sizeof(arr))
    {
        arr[i] = 2;
    }
    else
    {
        std::cout << "Arr out of bounds at: " << i << std::endl;
    }
}
code_fodder
  • 12,697
  • 10
  • 68
  • 123