Thread: Need help, whats wrong with this?

instead of return i, which is an integer, use return &arr[i]. Also in your function protoype you are telling it you are returning an integer, aka int Search. Use int * Search.

PHP Code:

#include<iostream.h>

int Search(int NUM_ELEMENTS,int arr[],int searchele) {
    for(int i=0;i<NUM_ELEMENTS;i++) {
        if(arr[i]==searchele)
            return i;
    }
    return NULL;  // Not sure this is valid return for int
}

int main() {
    int arr[] = {1,2,5,4};
    int *temp;
    temp=Search(4,arr,5);
    cout<<*temp;
    return(0);  // Should be return 0;
} 


Some comments added above. Multiple returns are bad ideas in general, and not sure that NULL is a valid return anyway. Try setting another variable to i in the if statement, then just returning that. Initialize that variable first, though, to a number that wouldn't be in a valid subscript range. That way, you can check for that value after the return in case searchele isn't found.

First off, what do you mean by this:
>...to find what spot a certain number in an array is.
What do you mean by "spot"? You can either have the element index number, or the memory address (in the form of a pointer).
Your code is getting confused between the two.

Here is your code with the necessary fixes. I have provided two versions, one which return a pointer to the int, and one which returns an index element number.

Code:

#include <iostream.h>

/* This version returns a pointer to the int in question
 * If not found, NULL is returned.
 */
int *Search(int NUM_ELEMENTS, int arr[], int searchele)
{
    for (int i = 0; i < NUM_ELEMENTS; i++)
    {
        if (arr[i] == searchele) return(&arr[i]);
    }

    return(NULL);
}

int main(void)
{
    int arr[] = { 1, 2, 5, 4 };
    int *temp;
    temp = Search(4, arr, 5);
    if (temp)
    	cout << "Found "<<*temp <<" at memory address " << temp <<endl;
    else
    	cout << "Not found" <<endl;
    return(0);
}

Code:

#include <iostream.h>

/* This version returns an int which is the index within the array.
 * If not found, -1 is returned.
 */
int Search(int NUM_ELEMENTS, int arr[], int searchele)
{
    for (int i = 0; i < NUM_ELEMENTS; i++)
    {
        if (arr[i] == searchele) return(i);
    }

    return(-1);
}

int main(void)
{
    int arr[] = { 1, 2, 5, 4 };
    int temp;
    temp = Search(4, arr, 5);
    if (temp >= 0)
    	cout << "Found "<<5 <<" at index " << temp <<endl;
    else
    	cout << "Not found" <<endl;
    return(0);
}

>return(0); // Should be return 0;
return (0); is valid.

>Multiple returns are bad ideas in general...
No, I don't believe they are. Depending on their usage, multiple returns are a good thing, as shown in the above example.

>Multiple returns are bad ideas in general...
No, I don't believe they are. Depending on their usage, multiple returns are a good thing, as shown in the above example.

I disagree with that. Maybe they're necessary in some cases, but they can lead to confusing tangles of code. Above, why not declare an integer, set it to -1, change it if found, then return x?

PHP Code:

   int x = -1;
   if yada yada
      x = y;
  return x; 


No need for multiple returns. In main, check if the returned value is -1.

>I disagree with that.
- We *could* argue all day and get no where with this, which is something I am not going to do But to keep it going for now...

>Maybe they're necessary in some cases, but they can lead to confusing tangles of code.
- Which is why I said "Depending on their usage". It's up to the programmer to ensure that they are used clearly and affectively. Burying them in the depth of code is obviously bad, but there are many examples where they can be put to good use.

>Above, why not declare an integer, set it to -1, change it if found, then return x?
- Yes, that is another way to do it. However, it means the use of another variable, which for the program's sake is unnecessary. It will also mean the code goes round the loop NUM_ELEMENTS times, which is pointless if it finds it's answer on the first iteration. This is a blatant waste of CPU To get round this, you'd need to add a break statement to terminate the loop early. Personally, I think all this bloats the code unnecessarily (in this particular snippet of code). Here's an example of both styles, side by side, so anyone can decide for themself:

Code:

/* Two version of the same thing! */
int Search(int NUM_ELEMENTS, int arr[], int searchele)
{
    int found = -1;
    for (int i = 0; i < NUM_ELEMENTS; i++)
    {
        if (arr[i] == searchele) 
        {
        	found = i;
        	break;
        }
    }
    return(found);
}

int Search(int NUM_ELEMENTS, int arr[], int searchele)
{
    for (int i = 0; i < NUM_ELEMENTS; i++)
    {
        if (arr[i] == searchele) return (i);
    }
    return(-1);
}