Looking through an array of objects

Question

I'm trying to write a method that looks through an array of objects for a certain color, which is also an object.

public Ghost findFirst(Color c){
        for (int i=0;i<ghosts.length;i++) {
            if (ghosts[i].getColor()==c)
            return ghosts[i];
        else
            return null;
            }  
    }

So if the color of a certain ghost matches color c, then return that ghost. However, I'm getting a dead code warning for i++. Whats wrong with my code? Oh also I'm getting a method error saying this function should return a ghost. I thought I am?

Answer 1

Because you return from the loop on the first iteration! So "i" never gets incremented. Either remove the else block completely or change the "return null" to "continue".

As a separate point, the == is checking referential equality, not object equality. It seems likely you should be using ".equals" instead

Answer 2

Its because of

else 
  return null;

!

Because of that return-Statement your Loop will only be executed once.

Answer 3

public Ghost findFirst(Color c){
    for (int i=0; i < ghosts.length; i++) {
        if (ghosts[i].getColor().equals(c))
            return ghosts[i];
    }  
    return null;
}

Answer 4

fixed code:

public Ghost findFirst(Color c){
        for (int i=0;i<ghosts.length;i++) {
            if (ghosts[i].getColor().equals(c))
               return ghosts[i];
        }
        return null;
    }

keep in mind that return terminates the function (including the loop obviously). So if you found the right color ghost - you return it (thus ending your search and never reaching the return null; line). If your for loop found nothing - you get to the last line and return null.

Answer 5

If I 'unroll' your loop, the code does something like:

public Ghost findFirst(Color c){
    if (ghosts[0].getColor()==c)
       return ghosts[0];
    else
       return null;  

    if (ghosts[1].getColor()==c)
       return ghosts[1];
    else
       return null; 

    if (ghosts[2].getColor()==c)
       return ghosts[2];
    else
       return null; 
    //etc...
}

Should be clear from this, that it will never reach the second if, it returns (breaks out of the function) at the first if, true or false.

Answer 6

Your multiple return may be a problem. Sometime it makes it simpler to have one return.

public Ghost findFirst(Color c) {
    Color color = null;
    for (int i=0;i<ghosts.length;i++) {
        if (ghosts[i].getColor().equals(c))
        color = ghosts[i];
    }
    return color;   
}