Stops at for loop

Question

When I am running my code, it always stops at the for loop and skips it.

public void assignCell()
{
    Prisoner prisoner = prisoners.get(id-1);
    for(Cell cell : cells)
    if(cell.isAvailable())
    {
        cell.allocate(prisoner);
        String bunk = null;
        if(cell.isEven())
        {
            bunk = "top bunk of cell";
        }
        else
        {
            bunk = "only bunk of cell";
        }
        System.out.println("\t\t" + prisoner.nameToString() + " is in the " + bunk + cell.toString());
    }

}

How can I fix this so it goes through?

Answer 1

Make sure cell.isAvailable() returns true when it is supposed to. The only way the loop could appear to be "skipped" is if the collection cells is empty or none of the cells are "available."

Answer 2

That would suggest that cells is empty. If it's not, we're just guessing - please post a complete program.

However, I would strongly urge you to add braces around your if statement¹:

public void assignCell()
{
    Prisoner prisoner = prisoners.get(id-1);
    for(Cell cell : cells)
    {
        if(cell.isAvailable())
        {
            cell.allocate(prisoner);
            String bunk = null;
            if(cell.isEven())
            {
                bunk = "top bunk of cell";
            }
            else
            {
                bunk = "only bunk of cell";
            }
            System.out.println("\t\t" + prisoner.nameToString() 
                               + " is in the " + bunk + cell);
        }
    }
}

In fact, I'd then try to reduce the nesting, and use the conditional operator too:

public void assignCell()
{
    Prisoner prisoner = prisoners.get(id-1);
    for(Cell cell : cells)
    {
        if(!cell.isAvailable())
        {
            continue;
        }
        cell.allocate(prisoner);
        String bunk = cell.isEven() ? "top bunk of cell" : "bottom bunk of cell";
        System.out.println("\t\t" + prisoner.nameToString() 
                           + " is in the " + bunk + cell);

    }
}

Oh, and you probably want a return or break statement there, otherwise a single prisoner will be assigned to all the available cells. Indeed, that may be happening with your first prisoner: check the output of the program very carefully!

---

¹ Another alternative is just to indent the if statement - but give some indication that you really did mean the if statement to be in the loop. Personally I find it helpful always to use braces, as then you can't accidentally add another statement after the first one which looks like it would be part of the loop, but isn't. Readability is king, IMO.

Answer 3

Are you sure your collection of cells isn't empty ? I would:

1. print the number of cells prior to looping (cells.size() if it's a collection, cells.length if it's an array)
2. print some cell info for each iteration, before you make any checks (clue: override toString() on the cell object and display availability etc.)

Answer 4

Even though the code looks correct, I would enclose the loop in braces. It makes it much easier to read.

Besides that, I would check to make sure that cells has items in it.

Answer 5

For a start I would put the contents of the for loop into {}

Answer 6

I'll mention this since no one else has: you should learn how to use the debugger that is bundled with your IDE. It can tell you if your cells array is empty. If you're using Eclipse, there are great debugging tutorial videos available here: http://eclipsetutorial.sourceforge.net/debugger.html