0

I'm trying to write a function that checks if an element is an array in C. I have a function here, but it always returns 0, so any debugging help would be appreciated:

#include <stdio.h>
int array[] = {1, 2, 3};
int *parray = &array;
int j;
int n = sizeof(array)/sizeof(array[0]);

int ifInList(int array[], int n){
    for (j = 0; j < n; j++){
        if (array[j] == n){
           return 1;
        }
    }
    return 0;
}

int main(){
    printf("The number is %d \n", ifInList(&array[n], 1));
    return 0;
}
thorium
  • 187
  • 8
  • 4
    `ifInList(&array[n], 1)` why? – babon Jul 05 '17 at 16:21
  • 1
    For one, you're missing a parameter to your function. You need the array, the array's size, and the element you're looking for – Alexander Jul 05 '17 at 16:22
  • 1
    you also have 2 things called 'n'. The sizeof the array at global scope and the number you are looking for at function scope. Change the array size global to array_size of something like that – pm100 Jul 05 '17 at 16:24
  • 1
    Possible duplicate of [Check if a value exist in a array](https://stackoverflow.com/questions/15094834/check-if-a-value-exist-in-a-array) – byxor Jul 05 '17 at 16:24
  • Also, try and avoid the use of *global* variable *unless absolutely necessary*. There is no reason not to declare all variables in `main()` and pass as parameters as required here. (some callback functions do need gobals,and there are other uses, but none apply here) – David C. Rankin Jul 05 '17 at 16:39
  • It is a bit ambiguous what you're looking for. Your description and code gives the impression that you want to return 1 if element is found and 0 otherwise. However, the printf statement gives another impression. What do you mean? – klutt Jul 05 '17 at 16:52

4 Answers4

3

You are searching incorrectly:

ifInList(&array[n], 1);

arr[0]    arr[1]            ....   a[n-1]    a[n]     
<----Search only these many elements --->     ^~~~You are searching unreserved address.

Accessing a[n] could result into bugs that are very difficult to debug. You are not supposed to do that at first place.

Correct code :

ifInList(array, sizeOfArray, valueToFind);  //searches in the complete array

for (j = 0; j < sizeOfArray; j++){  //Loop 
      if (array[j] == valueToFind){

Another important advice, try to avoid usage of global variables when they are not necessarily needed.

Saurav Sahu
  • 13,038
  • 6
  • 64
  • 79
3

Working code for checking if element n is in an array:

int ifInList(int array[], int size, int n) {
    int j;
    for(j=0; j<size; j++)
        if(array[i]==n)
            return 1;
    return 0;
}

You will need three parameters to do this. The array, the size and the element you're looking for.

Other things about your code:

Global variables are in general a bad habit. While you could (theoretically, maybe, but probably not) get away with having the array and the size of it as a global, there's absolutely no reason at all to let j be a global. It should be inside the function ifInList.

As a general rule:

If you're using global variables for convenience, you're doing something wrong.

klutt
  • 30,332
  • 17
  • 55
  • 95
3

There are two issues in your code:

  1. You are accessing a memory address which you have not reserved: array[n]
  2. Within ifInList you intend to use the name n for both the size of the array (your global variable) and the value you are looking for.

In particular:

Accessing a memory address you do not own

In this line:

printf("The number is %d \n", ifInList(&array[n], 1));

You are passing &array[n] as an argument to ifInList. This introduces two types of errors:

  1. Addressing error: array[n] does not exist. It refers to the n+1-th element of the array, a memory address which you have not reserved. In other words, you are "out of bounds".
  2. Logical error: you are not passing the address of the first element of the array. If you fixed it to, say, array[n-1], your function will only look at the last element of your array. In order to start searching from the first element, you need to pass either array or &array[0]; both are identical and refer to the memory address of the first element.

Conflicting variable names

First things first: global variables are almost always a bad idea, they should only be used in "quick and dirty" programs and/or for quick debugging purposes, as they can introduce confusion.

You are defining n as a global variable, which refers to the size of array. Nevertheless, within function ifInList, variable n has a different meaning: it's the key you are looking for. Therefore, if you are looking for "42" in a array with 3 elements, your function will look up to 42 different elements, instead of 3!

To avoid this confusion, give a different name to the key you are looking for:

int ifInList(int array[], int k){
    for (j = 0; j < n; j++){
        if (array[j] == k){
           return 1;
        }
    }
    return 0;
}

And the recommended approach is to provide the size with the key as arguments, thus eliminating completely the need for a global variable and any other potential conflicts. Furthermore, it's a clean programming practice. Note how I am also defining j within the function:

int ifInList(int array[], int n, int k){
    int j;
    for (j = 0; j < n; j++){
        if (array[j] == k){
           return 1;
        }
    }
    return 0;
}
Adama
  • 720
  • 2
  • 5
  • 23
2

While you already have good answers for your problem, continuing from my comment, always try and avoid the use of global variables. Declaring variables within proper scope avoids a number of potential problems and name collisions as your projects become larger. You can easily declare all of your variables within main() and pass them as parameters as required to make them available in other parts of your code. A quick example would be:

#include <stdio.h>

int ifinlist (int *array, size_t nelem, int n){

    for (size_t j = 0; j < nelem; j++)
        if (array[j] == n)
        return 1;

    return 0;
}

int main (void) {

    int array[] = {1, 3, 5},
        n = sizeof array / sizeof *array;

    for (int i = 0; i <= array[n - 1]; i++)
        printf ("The number '%d' %s present\n", 
            i, ifinlist (array, n, i) ? "is" : "is not");

    return 0;
}

Example Use/Output

Trivial output for the code would be:

$ ./bin/array_inlist
The number '0' is not present
The number '1' is present
The number '2' is not present
The number '3' is present
The number '4' is not present
The number '5' is present

Good luck with your coding.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85