Lack of searched element in array returns wrong value (pointers searching) in C++

Question

It's probably a stupid problem, but I can't get it right. It's supposed to look like that:

Input:
2            // amount of data sets
5            // amount of numbers in array
1 2 3 1 5    //array elements
3            //searched element index +1
4            //and so on
4 3 2 1
5

Output:
3
None

But if there is no searched value program returns "98779" instead of "NONE" I have no idea whats wrong. Here is the code

#include <iostream>
#include <algorithm>
using namespace std;

int main()
{
  int tab[100000];
  int x,y,z,elem;
  cin >> x;
  
    for(int i=0;i<x;i++)
    {
      cin >>y;
      for(int j=0;j<y;j++)
        {
          cin >> z;
            tab[j]=z;
        }
      cin >> elem;
      int n = sizeof(tab)/sizeof(tab[0]);
      auto itr = find(tab, tab + n, elem);
      
      if (itr != end(tab))
      {
        cout << distance(tab,itr)+1;
      }
      else 
      {
          cout << "NONE";
      }
    }
  return 0;
}

You'll be glad to hear you don't need anyone's help to figure this out, just a tool you already have: your debugger! This is exactly what a debugger is for. It [runs your program, one line at a time, and shows you what's happening](https://stackoverflow.com/questions/25385173/), this is something that's every C++ developer must know how to do. With your debugger's help you'll able to quickly find all problems in this and all future programs you write, without having to ask anyone for help. Have you tried using your debugger, already? If not, why not? What did your debugger show you? — Sam Varshavchik, Jul 21 '22 at 13:52
`int n = sizeof(tab)/sizeof(tab[0]);` is always `100000`. I don't think that's what you want `n` to be. — 001, Jul 21 '22 at 13:54
*I have no idea whats wrong* -- If you wrote this code, you should never be in the position of not having an idea what is wrong. Every line of code you write, you must know what the intent of that line of code is. If the program doesn't give the right results, then debug the code to find out where the program goes against the plan you had in mind when you wrote the program. — PaulMcKenzie, Jul 21 '22 at 13:54
`int tab[100000];` -- Declarations like this are lazy (seen it too many times with beginners), and a big part of what is wrong. What if there are only 10 items instead of 100000? Instead, learn to use `std::vector tab;`, where you dynamically add elements to tab, and thus you know what the actual number of elements will be. Doing just that would have alleviated the errors that you see mentioned in the answers below. — PaulMcKenzie, Jul 21 '22 at 13:58
Your program told you that it found `3` at `tab[98779]`. Did you check the values in `tab` to see if that was accurate? — JaMiT, Jul 21 '22 at 14:19
@OP [Using std::vector shows the entire issue](https://godbolt.org/z/ov4znWjT8) is due to the use of declaring gigantic arrays, way beyond the number of actual elements that exist. It is basically the same code you wrote, but using the proper constructs. — PaulMcKenzie, Jul 21 '22 at 14:29

Vlad from Moscow · Answer 1 · 2022-07-21T14:38:10.940

For starters you need to include header <iterator>

#include <iterator>

These statements

  int n = sizeof(tab)/sizeof(tab[0]);
  auto itr = find(tab, tab + n, elem);

are incorrect.

You entered y elements in the loop

  cin >>y;
  for(int j=0;j<y;j++)
    {
      cin >> z;
        tab[j]=z;
    }

So the variable n must be equal to y.

So you have to write

  int n = y;

  auto itr = find(tab, tab + n, elem);
  
  if (itr != tab + n )
  {
    cout << distance(tab,itr)+1;
  }
  else 
  {
      cout << "NONE";
  }

Pay attention to that instead of the array it would be better to use the standard container std::vector<int> as shown in the demonstration program below.

#include <iostream>
#include <vector>
#include <iterator>
#include <algorithm>

int main()
{
    unsigned int count = 0;

    if ( std::cin >> count )
    {
        while ( count-- )
        {
            unsigned int n = 0;

            if ( std::cin >> n )
            {
                std::vector<int> v;
                v.reserve( n );

                std::copy_n( std::istream_iterator<int>( std::cin ), n,
                             std::back_inserter( v ) );

                int elem = 0;
                std::cin >> elem;

                auto it = std::find( std::begin( v ), std::end( v ), elem );

                if ( it != std::end( v ) )
                {
                    std::cout << std::distance( std::begin( v ), it ) + 1 << '\n';
                }
                else
                {
                    std::cout << "NONE\n";
                }
            }
        }
    }
}

score 0 · Answer 2 · answered Jul 21 '22 at 13:55

0

The lines

int n = sizeof(tab)/sizeof(tab[0]);

and

if (itr != end(tab))

are bad.

The lines mean to search from the whole array despite of only the first few elements are initialized.

The lines should be

int n = y;

and

if (itr != tab + n)

answered Jul 21 '22 at 13:55

MikeCAT

73,922
11
45
70

you can use `y`, no need for another variable `n` – 463035818_is_not_an_ai Jul 21 '22 at 13:55

Lack of searched element in array returns wrong value (pointers searching) in C++

2 Answers2