Dead code with arrays?

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

what should "dead code" be? Add some comments to your code otherwise no one will read this. — Stefan Sprenger, Jan 23 '14 at 15:12
As an aside, you have returns in your for loops. That's not going to finish going through your arrays. Which could be filled using java.util.Arrays.fill(). — nitind, Jan 23 '14 at 15:24

eltabo · Accepted Answer · 2014-01-23T16:10:17.447

4

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.

edited Jan 23 '14 at 16:10

answered Jan 23 '14 at 15:25

eltabo

3,749
1
21
33

So i assume I would decrement and set the array indexs to 1 then return the value? – 123a169 Jan 23 '14 at 15:56

score 1 · Answer 2 · answered Jan 23 '14 at 15:25

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.

score 1 · Answer 3 · answered Jan 23 '14 at 16:13

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.

Dead code with arrays?

EDIT

3 Answers3