When there is two for() loop, the second one doesn't work.

Hi,

Here are my for() loops :

public void showMovementCase(){
 int movePlusAttack = moveAllowed+attackDistance;
 int twiceMoveAllowed = (moveAllowed)*2;
 for(int i = 0; i <= movePlusAttack*2; i++){
  for(int j = 0; j <= movePlusAttack*2;j++){
   boolean a = movePlusAttack <= j+i && movePlusAttack >= j-i && i <= movePlusAttack;
   boolean b = movePlusAttack <= j+i && movePlusAttack >= i-j && i > movePlusAttack && j <= movePlusAttack;
   boolean c = movePlusAttack*3 >= j+i && movePlusAttack >= j-i && i > movePlusAttack &&  j >= movePlusAttack;
   if(a || b || c){
    try{
     actionSquare[i][j] = new JLabel();
     actionSquare[i][j].setIcon(redsquare);
     actionSquare[i][j].setBounds(sprite.getX()+(i-movePlusAttack)*16,sprite.getY()+(j-movePlusAttack)*16, 16, 16);
     panel.add(actionSquare[i][j], new Integer(1));
    }
    catch(ArrayIndexOutOfBoundsException e){System.out.println("red :" + e);}
   }
  }
 }
 for(int x = 0; x <= twiceMoveAllowed; x++){
  for(int y = 0; y <= twiceMoveAllowed;y++){
   boolean a = moveAllowed <= y+x && moveAllowed >= y-x && x <= moveAllowed;
   boolean b = moveAllowed <= y+x && moveAllowed >= x-y && x > moveAllowed && y <= moveAllowed;
   boolean c = moveAllowed*3 >= y+x && moveAllowed >= y-x && x > moveAllowed &&  y >= moveAllowed;
   if(a || b || c){
    try{
     actionSquare[x][y].setIcon(bluesquare);
     System.out.println("Coucou !");
     actionSquare[x][y].addMouseListener(mouse);
     panel.repaint();
     panel.revalidate();
    }
    catch(ArrayIndexOutOfBoundsException e){System.out.println("blue :" + e); }
   }
  }
 }
}

if this.attackDistance is different of 0, then the second loop doesn't work (it seems to stop at the .setIcon() command).

Do you know a way to fix this ?

Thanks for reading.

Edit:

with :

                try{
                    actionSquare[x][y].setIcon(bluesquare);
                    System.out.println("Coucou !");
[...]
                }

On the second loop, nothing is printed.

but with :

            try{
                System.out.println("Coucou !");
                actionSquare[x][y].setIcon(bluesquare);

[...] }

"Coucou !" is printed once. That's why I said that "it seems to stop at the .setIcon() command" I should have said that sooner, sorry.

From stackoverflow

• Hmmm... where to begin...

  I would first suggest putting something (System.err.println(...)?) inside of your catch blocks. Or just commenting them out entirely so you'd see the full stacktrace. What if you're hitting an exception and just not seeing it?

• catch(ArrayIndexOutOfBoundsException e){}
  

  This is a bad practice for two reasons:

  1. You should never catch RuntimeException. It is just a very helpful indicator for errors in code logic (i.e. developer errors) which ought be solved by writing good code.
  2. You should never ignore e unless you know perfectly what you're doing. Add at least an e.printStackTrace() so that you at least know that something failed.
  BalusC : Again? Hey there, downvoting without commenting == lame. What's up, doc?

• This is a very, very bad idea:

  catch(ArrayIndexOutOfBoundsException e){}
  

  You effectively tell the JVM to ignore any problems with your arrays that it detects. And worse than that: you don't even print anything when that happens.

  Put at least a e.printStackTrace() in there to see if a problem occurs and where.

  And as a further step: fix your array access to not exceed any limits. Catching an ArrayIndexOutOfBoundsException is a terribly bad idea. Avoid having it thrown at all!

  Edmund : Given the overall algorithm it looks like the intention is to ignore any cells outside the valid range. So at most I'd recommend putting a comment "//ignore" in the exception handler. Though a better approach is to change the algorithm to check i and j and skip the step if they're outside.

  Joachim Sauer : Putting an "`/ignore`" in the catch-block of an unavoidable Exception that you want to ignore is better than an empty catch-block for sure. But using an `ArrayIndexOutOfBoundsException` for flow control is very, very ugly and definitely avoidable.

• Here are a few tips:

  • don't catch exceptions and do nothing with them. That's what you are doing here in both loops, and so it's normal you don't see the error message.

  • anytime you see long statements like you have, it should be a hint that you could refactor it. For example, create a separate method that validates whether or not you're going to do something in your loop, and then inside the main method you'd call it like if(shouldPerformAction())

  • consider using less than 8 spaces for indentation. This just eats up your screen real estate.

  • consider making computations before the loops instead of inside the loop conditions, if the computation is supposed to be fixed (for example this.moveAllowed*2)

  • imho, no point in prefixing all your methods/fields with this, it just clutters everything. Just call the methods directly.

• I cleaned up your code for you. Generally, when you have two sections of code that are supposed to be doing the exact same thing, but are not, then rolling them into one method can eliminate that possibility.

  public void showMovementCase(){
      // probably want to remove anything left over from the last invocation
      panel.removeAll();
      for (JLabel[] array : actionSquare) Arrays.fill(array, null);
       
      colorSquares(moveAllowed + attackDistance, redsquare, null);
      colorSquares(moveAllowed * 2, bluesquare, mouse);
       
      for (int x = 0; x < actionSquare.length; x++)
          for (int y = 0; y < actionSquare[x].length; y++)
              if (actionSquare[x][y] != null) panel.add(actionSquare[x][y], 1);
  }
   
  private void colorSquares(int move, Icon color, MouseListener mouse) {
      int xMax = Math.min(2 * move, actionSquare.length);
      int yMax = Math.min(2 * move, actionSquare[0].length);
      for (int x = 0; x < xMax; x++) {
          for (int y = 0; y < yMax; y++) {
              if (isLegal(x, y, move)) {
                  if (actionSquare[x][y] == null)
                      actionSquare[x][y] = new JLabel();
                  actionSquare[x][y].setIcon(color);
                  actionSquare[x][y].setBounds(
                          sprite.getX() + (x - move) * 16,
                          sprite.getY() + (y - move) * 16, 16, 16 );
                  if (mouse != null) actionSquare[x][y].addMouseListener(mouse);
              }
          }
      }
  }
   
  private static boolean isLegal(int x, int y, int move) {
      // informative comment explaining why this mess makes sense
      if (move <= y+x && move >= y-x && x <= move) return true;
      // informative comment explaining why this mess makes sense
      if (move <= y+x && move >= x-y && x > move && y <= move) return true;
      // informative comment explaining why this mess makes sense
      if (move * 3 >= y+x && move >= y-x && x > move &&  y >= move) return true;
       
      return false;
  }

  Cheshire : Thanks a lot, that fixed it.