0

I have a very simple program which is given below. I have inserted couts to know which line gets executed.

int main(void) {
  int n_in = 0;
  int keys = 0;
  cin>>n_in;
  long long in_array[n_in];

  for(int i=0; i<n_in; i++){
    cin>>in_array[i];
  }
  cout<<"Executed"; 
  cin>>keys;
  cout<<"Executed"<<" "<<keys;
  int index[keys];
  long long key_array[keys];
  cout<<"Executed";
  for(int j=0; j<keys; j++){
    cin>>key_array[j];
    cout<<"Iteration" <<j<<"complete" ;
  }
  cout<<"Executed";
  //bin_search(in_array, n_in, key_array, keys, index);

  for(int i=0; i<keys; i++){
    cout<<index[i]<<" " ;
  }
  return 0;

}

The screenshot is given:

enter image description here

As you can see from the image, the last iteration never completes and I don't know why. The numbers after iteration0complete etc. are inputs.

Can someone please explain whats happening?

Biffen
  • 6,249
  • 6
  • 28
  • 36
Plutonium smuggler
  • 329
  • 5
  • 6
  • 17
  • http://ericlippert.com/2014/03/05/how-to-debug-small-programs/ – Biffen Mar 29 '16 at 06:41
  • 4
    Technically your code isn't valid C++, as C++ doesn't have [variable-length arrays](http://en.wikipedia.org/wiki/Variable-length_array). It works because of a compiler extension, to be portable you should be using [`std::vector`](http://en.cppreference.com/w/cpp/container/vector) instead. – Some programmer dude Mar 29 '16 at 06:42
  • @silflow No, technically it is not standards compliant. It requires a compiler extension. But it is not UB. – juanchopanza Mar 29 '16 at 06:42
  • Anyway, it looks like a classic case of out of bounds access, but better post an [mcve]. It is easier if you remove the stdin inputs too, so you can rule out problems with input. – juanchopanza Mar 29 '16 at 06:44
  • Use Vectors instead of array if you need dynamic size – usamazf Mar 29 '16 at 06:44
  • @juanchopanza. you are correct. It's not ISO C++. – Silvano Brugnoni Mar 29 '16 at 06:45
  • 1
    And please try to add some newlines to your output, so it's easier to see what's input and what's output. And perhaps not call all debugging output `"Executed"`? – Some programmer dude Mar 29 '16 at 06:45
  • 1
    What have you learned from your session with the debugger? – n. m. could be an AI Mar 29 '16 at 06:47
  • 1
    Technically, since you're not initialising the `index` array, the last loop makes your program undefined. – molbdnilo Mar 29 '16 at 09:49
  • Also, for print-debugging, it's important to flush the output buffer *or* print to `cerr` instead of `cout`. – molbdnilo Mar 29 '16 at 09:56
  • @molbdnilo. Agreed. That bin_search procedure (which is commented) was modifying index array. But before that there is one more "Executed" which never executes. I guess that is because last iteration is not getting complete for key_array. At the beginning even I thought that I was trying to access an illegal index. But if you change the condition from j – Plutonium smuggler Mar 30 '16 at 07:17
  • @JoachimPileborg. Thanks for the suggestion. I'll try to rewrite the program using vectors, though I'm still puzzled as to where it is going wrong. It would be helpful if you could explain that. – Plutonium smuggler Mar 30 '16 at 07:21

1 Answers1

1

Here is a modified version of your code, as mentioned in the comments i used vectors instaed of arrays, because you can use vector without knowing the size of it, which is the opposite of declaring arrays, at compile time the compiler must know the size of the array, but you define an array and you give it a size which is not a constant at compile time, i commented the part of the code where you use the array index[] because i dont know why you are printing what the array has, while it is empty, you dont have anything inside it you just declared it.

here is the code hope it fulfill your needs.

#include<string>
#include<vector>
#include<iostream>



using namespace std;


int main(){


    int n_in = 0;
  int keys = 0;
  cin>>n_in;
  vector<long long> in_array;

  for(int i=0; i<n_in; i++){
      int k =0;
      cin >> k;

    in_array.push_back(k);
  }
  cout<<"Executed"; 
  cin>>keys;
  cout<<"Executed"<<" "<<keys;

  vector<int> index;
  vector<long long> key_array;
  cout<<"Executed";

  for(int j=0; j<keys; j++){
      int p =0;

    cin>>p ;
        key_array.push_back(p);
    cout<<"Iteration" <<j<<"complete" ;
  }
  cout<<"Executed";
  //bin_search(in_array, n_in, key_array, keys, index);

  /*for(int i=0; i<keys; i++){
      int m =0;

    cout<<index[i]<<" " ;
  }
*/

    return 0;
}

Edit: What you said in the comment is true in C, but in c++ the compiler need to have the exact size of the array when compiling, otherwise you will have to use the new operator to dynamically allocate memory.

For example:

int MyArray[5]; // correct

or

const int ARRAY_SIZE = 6;
int MyArray[ARRAY_SIZE]; // correct

but

int ArraySize = 5;
int MyArray[ArraySize]; // incorrect

To sum it up:

The number of elements of an array, must be a constant expression . If you need variable bounds, use a vector.

Mourad
  • 474
  • 4
  • 10
  • I just use it because my console pop up and go quickly, but thanks for the info – Mourad Mar 29 '16 at 07:41
  • @Mourad. I appreciate that but i wanted to build the code myself. One thing i didnt understand. You said size of array must be constant at compile time. I've been doing this in C and it always worked. Even here, the first array which is "in_array" is defined like that and it works. Can you explain this ? – Plutonium smuggler Mar 30 '16 at 07:10
  • 1
    Please check the Edit – Mourad Mar 30 '16 at 07:34
  • @Mourad. Thnx. I rewrote the program using vectors. It now works. – Plutonium smuggler Apr 07 '16 at 15:51