Question

I am getting a "Dead Code" warning on my battleship game. I am testing if a ship can be placed and am seeing different direction that the ship will face if it can be placed.

I then get input using In.class and after checking if it can be placed with a boolean (set to true/false when checking directions for placement) I use a 2D array of ints and set all of the locations to 1 (Starting location then + length (given as a parameter) in direction specified)

my .java file is here ShipProperties.java

if possible please keep answers to a beginners skill level (basic i/o and arrays, im pretty good with logic)

EDIT

I got it fixed and it now works as intended!

Just put the return in a if/else inside the loop

for(int i = length; i > 0; i--)
{
 grid[startLocX][startLocY + i] = 1;
 if(i == 0)
 {
   return grid;
 }
}
Was it helpful?

Solution

In your code,

    for(int i = length; i > 0; i--)                               //Dead Code
    {
      grid[startLocX - i][startLocY] = 1;
      return grid;
    }

decrement in loop is never executed because in first iteration of the loop your method return a value, so never do a second iteration. Actually your code is same as:

    if(length > 0)                              
    {
      grid[startLocX - length][startLocY] = 1;
      return grid;
    }

Hope it helps.

OTHER TIPS

Your loops all return in the first iteration. E.g.

for(int i = length; i > 0; i--)
{
    grid[startLocX - i][startLocY] = 1;
    return grid;
}

does the same as

int i = length
if(i > 0)
{
    grid[startLocX - i][startLocY] = 1;
    return grid;
}

so your loop is unecessary. The actual dead code is i-- which never gets executed.

I think you want to move the return statement from inside the for loops to the end of the if(canPlace == true) clause. I'd also recommend you de-clutter the code a bit to make it more readable:

if(canPlace == true)
{
    for(int i = length; i > 0; i--)                            
    { // a little less efficient perhaps (will be optimized by JIT if it's critical), but a lot more readable for sure.
        switch (ans) {
        case "Left":
            grid[startLocX - i][startLocY] = 1;
            break;
        case "Down":
            grid[startLocX][startLocY - i] = 1;
            break;
        case "Right":
            grid[startLocX + i][startLocY] = 1;
            break;
        case "Up":
            grid[startLocX][startLocY + i] = 1;
            break;
        default:
            throw new IllegalArgumentException("huh? " + ans);
        }
    }
}
// no need for an else clause since you return the grid anyway
return grid;

note that I'm using string switch-case (new in java 7) and check for unexpected argument.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top