-1

I am stuck and a simple yet crazy search algorithm. I want to find the index of an element from an array calling a function. The problem is that although the element is already there, the function returns -1; which is supposed to return only if the element is not there. This is the code:

#include <iostream>
using namespace std;


int search(int arr[], int size, int val, bool left) {
  int res;
  for (int i = 0; i < size; i++) {
    if(val == arr[i]) {
      res = i;
      break;
    }
    else if(val != arr[i]) {
      res = -1;
    }
  }
  return -1;
}

int main() {

  int arr[] = {1, 4, 6, 5, 2, 7, 10, 4};
  int size = sizeof(arr) / sizeof(arr[0]);

 cout << search(arr,size,6, true) << endl;



  return 0;
}

Thanks in advance.

Zaki Sediqyar
  • 41
  • 7
  • Maybe that `break` should be a `return ...`... – Bob__ Oct 11 '19 at 06:22
  • You should return `res`, not -1... – Aconcagua Oct 11 '19 at 06:22
  • you need to return res instead of -1 – Matriac Oct 11 '19 at 06:22
  • 1
    Be aware that the else inside the for loop is executed every time a test fails, assigning -1 again and again. You can do it more efficiently by initialising `res` with -1 before the loop and just drop the else. – Aconcagua Oct 11 '19 at 06:24
  • Have you tried debugging yet? Try to perform your algorithm with pen and paper by hand. – MrSmith42 Oct 11 '19 at 06:28
  • About [using namespace std;](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)... – Aconcagua Oct 11 '19 at 06:28
  • Oh Crap!!! I didn't notice that -1. Thank you @Aconcagua for the tip. I didn't know the else execution inside a for loop after every fail. – Zaki Sediqyar Oct 11 '19 at 06:29
  • Yet another point: `if(X == Y) {} else if(X != Y) {}` – second test is *complementary* to first one, so if first one fails, second one cannot. So in this case, just drop the second test: `if(X == Y) {} else {}` (in cases you yet need the else, unlike as in question). – Aconcagua Oct 11 '19 at 06:36

2 Answers2

0

Try Return res from the function

#include <iostream>
using std::cout;
using std::cin;
using std::endl;
int search(int arr[], int size, int val, bool left) {
  int res = -1;
  for (int i = 0; i < size; i++) {
    if(val == arr[i]) {
      res = i;
      break;
    }
  }
  return res;
}

int main() {

  int arr[] = {1, 4, 6, 5, 2, 7, 10, 4};
  int size = sizeof(arr) / sizeof(arr[0]);

 cout << search(arr,size,6, true) << endl;

  return 0;
}
Kiran Thilak
  • 443
  • 5
  • 15
0
for (int i = 0; i < size; i++) {
if(val == arr[i]) {
res = i;
break;
}
else if(val != arr[i]) {
res = -1;
}
}
return -1;

You are returning -1 outside of for loop even when the element is present.

Initialize res variable above for loop and return it res = -1

After for loop

return res