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?

Try indenting your code properly, then you'll see for yourself. But basically, you're defeating your for loop, if the first color isn't what you're looking for, you are returning null, and not even looking at all the other colors. — Jack Leow
– Jack Leow, Commented Oct 16, 2010 at 20:57
As I had answered earlier on, you will need to use equals() when comparing Color objects. — BoltClock
– BoltClock, Commented Oct 16, 2010 at 21:02

oxbow_lakes · Accepted Answer · 2010-10-16 20:56:31Z

7

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

answered Oct 16, 2010 at 20:56

oxbow_lakes

135k56 gold badges323 silver badges451 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

oxbow_lakes Over a year ago

I've amended my answer. As a point of interest, what is giving you the dead code warning? Your IDE? Which are you using?

Snowman Over a year ago

Ya I fixed the .equals thing. I'm using eclipse and i++ is highlited yellow with a dead code warning. I was given instructions to return null if nothing was found though. How can I still implement null in my case?

Snowman Over a year ago

Also when I removed the else block, I'm still getting a must return Ghost object error

corsiKa Over a year ago

fprime, you need to move the return null outside of the loop (see SKip Head's code example below). Basically your logic needs to be this: "I'm going to go through this array to see if I can find the ghost of the color I want. If I get through the entire list, and I still have not returned a ghost, then I should return null." ... Instead, you have "For each ghost in the list, I will return the ghost if it is that color, and null if it isn't." The problem with that is you can only return once, and you're forcing yourself to return right away. (Since you return before the end, i++ never happens).

Tom Teman · Accepted Answer · 2010-10-16 21:18:42Z

3

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.

edited Oct 16, 2010 at 21:18

answered Oct 16, 2010 at 21:08

Tom Teman

2,0133 gold badges29 silver badges43 bronze badges

1 Comment

Snowman Over a year ago

Oh ok, I didnt know return ends a loop. I did a messier method in my fix above, using booleans, which probably wasnt neccessary

imbaer · Accepted Answer · 2010-10-16 20:58:26Z

2

Its because of

else 
  return null;

!

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

answered Oct 16, 2010 at 20:58

imbaer

5543 silver badges22 bronze badges

Comments

Skip Head · Accepted Answer · 2010-10-16 21:08:39Z

2

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

answered Oct 16, 2010 at 21:08

Skip Head

7,8202 gold badges33 silver badges34 bronze badges

Comments

Ishtar · Accepted Answer · 2010-10-16 21:09:23Z

1

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.

answered Oct 16, 2010 at 21:09

Ishtar

11.7k1 gold badge27 silver badges31 bronze badges

Comments

fastcodejava · Accepted Answer · 2010-10-16 21:25:26Z

0

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;   
}

answered Oct 16, 2010 at 21:25

fastcodejava

41.3k31 gold badges142 silver badges192 bronze badges

Collectives™ on Stack Overflow

Looking through an array of objects

6 Answers 6

4 Comments

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

4 Comments

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related