Bad Color Referencing?

Question

I am in the middle of designing a game very similar to Flow free. I have run into an issue in my code where I want to be able to click on a circle in the grid, be able to gauge its color, and then check if the color is white. If the color is white, I will then print error, which is a placeholder for now. However, even when I click on non-white tiles, I still see the error message being printed. What Am I doing wrong?

My Code:

import javax.swing.JFrame; //JFrame Class
import java.awt.Canvas;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Shape;
import java.util.ArrayList;
import javax.swing.JPanel;
import java.awt.geom.Ellipse2D;
import java.awt.Graphics2D;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;


public class DriverV2 extends JPanel {


static ArrayList<Shape> circles;
static ArrayList<Color> colors;

public DriverV2() {
   circles = new ArrayList<Shape>();
   colors = new ArrayList<Color>();
   colors.add( Color.CYAN);//1
   colors.add( Color.BLUE);//2
   colors.add( Color.WHITE);//3
   colors.add( Color.WHITE);//4
   colors.add( Color.GREEN);//5
   colors.add( Color.WHITE);//6
   colors.add( Color.CYAN);//7
   colors.add( Color.WHITE);//8
   colors.add( Color.WHITE);//9
   colors.add( Color.RED);//10
   colors.add( Color.WHITE);//11
   colors.add( Color.YELLOW);//12
   colors.add( Color.WHITE);//13
   colors.add( Color.GREEN);//14
   colors.add( Color.WHITE);//15
   colors.add( Color.WHITE);//16
   colors.add( Color.BLUE);//17
   colors.add( Color.WHITE);//18
   colors.add( Color.RED);//19
   colors.add( Color.WHITE);//20
   colors.add( Color.WHITE);//21
   colors.add( Color.WHITE);//22
   colors.add( Color.WHITE);//23
   colors.add( Color.WHITE);//24
   colors.add( Color.YELLOW);//25
      int rows = 5;

   for (int y=0;y< rows;y++)
   {
      for (int x=0;x<rows;x++)
    {
        circles.add( new Ellipse2D.Double((x + 1) * 150, (y + 1) *150, 100, 100) );
    }
}
}

protected void paintComponent(Graphics g)
{
    super.paintComponent(g);
    Graphics2D g2d = (Graphics2D)g;
    Color foreground = g2d.getColor();



   int h = 0;
    for (Shape shape : circles)
    {

        g2d.setColor( colors.get(h));
        g2d.draw( shape );
        g2d.fill(shape);
        h++;
    }
}
public static boolean checkLinearity(int index1, int index2){
   if((index1%5) == (index2 %5)) { //vertical line check
      return true;
   }
   if(Math.abs(index1-index2) < 5) { //horizantal line check
      return true;
   }
   else {
      return false;
   }

}
static int counter = 0;
static int index1 = 0;
static int index2 = 0;
public static void main(String[] args)
      {

      JFrame f = new JFrame("Flow"); //new JFrame
      DriverV2 t = new DriverV2();
      f.setSize(900,900);//sets size of frame
      //  f.setLocation(100,50);//sets location of frame
      f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);//sets default close operation
      //  f.setContentPane(new Panel()); //sets content pane
      f.setVisible(true);//makes panel visible
      f.add(t);
      f.addMouseListener( new MouseAdapter() {
         public void mouseClicked(MouseEvent e) {
            int x = e.getX();
            int y = e.getY();

         for (Shape shape: circles) {

            if (shape.contains(x,y)) {
               counter++;
               if(counter % 2 == 1) {
                  index1 = circles.indexOf(shape)+1;
                  if(Color.WHITE.equals(colors.get(index1))) {
                     counter--;
                     System.out.println("error");
                  }
               }
               else{
                  index2 = circles.indexOf(shape) +1;
                  if(checkLinearity(index1, index2) == true) {
                     //I want to be able to draw a rectange here
                  }
               }


         }
}
         }
      });

}
}

Answer 1

Your problem seems another example of why many people (including me) advocate to always add curly braces (even on 1-statement blocks - they might not stay that way):

Have a look at this snippet:

if (shape.contains(x,y)) 
   counter++;
     if(counter % 2 == 1) {
      ...

It doesn't do what you think, i.e. if you hit the shape counter get's increased but the rest of the checks is performed for every shape. That's because to the compiler the code looks like this:

if (shape.contains(x,y)) 
   counter++;
if(counter % 2 == 1) {
  ...

Thus, when you hit a circle counter will be 1 and hence counter % 2 == 1 will be true for every circle being checked afterwards.

So, add curly braces around the indented code. Additionally you might want to break the loop once you've detected a hit since you're probably not interested in check all the other circles as well.

You might also want to introduce a class Circle which might look like this:

class Circle {
  Shape shape;
  Color color;

  ... //additional properties, e.g. position, and methods

  public boolean wasHit(int x, int y) {
    return shape.contains(x,y);
  }

  public boolean isValid() {
    return Color.WHITE.equals( color );
  }
}

Then you'd iterate over those like this:

for( Circle circle : circles ) {
  if( circle.wasHit(x, y) {
    if( circle.isValid() ) {
      //do what you'd need here, e.g. change the shape to a rectangle
    } else {
      //error
    }

    //break the loop since other circles can't be hit or we're not interested in those
    break;
  }
}