I need a fresh pair of eyes for debugging

Question

The purpose of this assignment was to create a Field and Robot class and objects of those classes.

The single field object is limited to a square of points from (0, 0) to (50, 50), and contains 3 pieces of gold and 3 bombs.

Three robot objects search the field (one after another) for gold from left to right (0, 0) to (0, 50) and descend through the field (1, 0) to 1, 50) and so on.

The robots are destroyed by bombs that are places by input from the user. Once the gold is collected it cannot be picked up by another robot, and one a bomb explodes it does not do so again.

This is my attempt at solving the problem so far, I am continuing to work on it, but would appreciate a second pair of eyes on it to catch something im missing. The program compiles, but the bombs and gold arent being "found" correctly and the output states that the following robots die on the same bomb as the one before it. Also, there are several pieces of code removed by comments, I did this to test different parts of the program. I think this section is where I'm having trouble. The methods field.alreadyFound() and field.alreadyBombed() return a boolean with the value true. My if statements should be saying if the gold/bomb has already been found, ignore it.

while(x <= 50 && y <= 50 && alive2 == true)
{   foundGold1 = robot2.look(field.locateGold1());
    foundGold2 = robot2.look(field.locateGold2());
    foundGold3 = robot2.look(field.locateGold3());
    foundBomb1 = robot2.look(field.locateBomb1()); 
    foundBomb2 = robot2.look(field.locateBomb2());
    foundBomb3 = robot2.look(field.locateBomb3());

    if (foundGold1 == true)
    {
        if (field.alreadyFound1() == true) 
        { 
        }
        else
        {
            robot2.addGold();
            field.foundGold1();
        }
    }
    if (foundGold2 == true)
    {
        if (field.alreadyFound2() == true) 
        { 
        }
        else
        {
            robot2.addGold();
            field.foundGold2();
        }
    }
    if (foundGold3 == true)
    {
        if (field.alreadyFound3() == true) 
        { 
        }
        else
        {
            robot2.addGold();
            field.foundGold3();
        }
    }
    if (foundBomb1 == true)
    {
        if (field.alreadyBombed1() == true)
        {
        }
        else
        {
            alive2 = false;
        }
    }
    if (foundBomb2 == true)
    {
        if (field.alreadyBombed2() == true)
        {
        }
        else
        {
            alive2 = false;
        }
    }
    if (foundBomb3 == true)
    {
        if (field.alreadyBombed3() == true)
        {
        }
        else
        {
            alive2 = false;
        }
    }
    y = y + 1;
    robot2.setLocation(x, y);
    //System.out.println(y);

    if (y == 50)
    {
        x = x + 1;
        y = 0;
    }    

Answer 1

I think some of your code is a bit too verbose and fluffy to let you see what's really going on. Take this small section:

if (foundGold1 == true)
{
    if (field.alreadyFound1() == true) 
    { 
    }
    else
    {
        robot2.addGold();
        field.foundGold1();
    }
}

It could be written like this:

if (foundGold1) {
    if (! field.alreadyFound1()) {
        robot2.addGold();
        field.foundGold1();
    }
}

Depending upon how this code will "grow" over time, you might want to simplify it a little further:

if (foundGold1 && ! field.alreadyFound1()) {
    robot2.addGold();
    field.foundGold1();
}

Please note Matthew Cox's warning: young programmers often get caught up in trying to minify their code as much as they possibly can (Look I did it in 2 lines of code instead of 3!) -- I'm not arguing for the reformatted layout because it is smaller but because I believe this is easier to read. Depending upon the complexity of future code, I might give exactly the opposite advice to break apart conditions. The top priority is writing legible code. (The compiler can figure out nearly anything you throw at it -- you have to be able to understand it too.)

His further advice to use arrays for foundBomb[], foundGold[], robot[], etc. is superb -- and is best paired with using function arguments instead of writing multiple functions.

Instead of field.alreadyFound1(), field.alreadyFound2(), etc, try: field.alreadyFound(int something) -- when you call them, it'll be field.alreadyFound(2). This lets you cut the number of functions by three (which means you won't have to fix bugs three times in each function) and makes it easier to grow the program to five or six fields, piles of gold, bombs, robots, etc. It's just a parameter to a function then.

Answer 2

I can see one potential problem although it probably isn't related to your issue.

I would assume that if a robot hit a bomb that it should not be able to continue and set off another bomb. Your code potentially allows a single robot to set off all three bombs I think.

Answer 3

There is too much unaccounted for logic going on here to know exactly why your program is failing.

However, just some thoughts, suggestions, and comments that made aid in solving your problem.

• Utilize Open/Close brackets, young programmers often get caught up in trying to minify their code as much as they possibly can (Look I did it in 2 lines of code instead of 3)!
• You don't need to check if a boolean variable is == true/false. You can simply say if (foundGold1) OR if you want to see if its false you say if (!foundGold1)

Those things said,

The issue really boils down to the you seem to have a set of logic that includes things like

• foundGold1, foundGold2, foundGold3
• robot2 (is there a robot1 and robot3)?
• foundBomb1, foundBomb2, foundBomb3
• alive2 (is there an alive1 and alive3)?

You could be storing these in arrays to really reduce the checking necessary, however I suppose this somewhat depends on some details you haven't revealed.