Bouncing square in a box

Question

I wrote my first animation in Java FX and would like a code review.

I created StackPane with Rectangle. Rectangle starts to move inside StackPane to right-bottom direction and changes direction after hitting the StackPane bound.

Please review the code that moves Rectangle. Is it a correct implementation of a Java FX animation?

//...  
new RectangleMover(pane, shape);  
// ...

RectangleMover.java

public class RectangleMover extends Timer
{
   private final StackPane pane;
   private final Rectangle shape;
   private boolean moveRight = true;
   private boolean moveBottom = true;


   public RectangleMover(StackPane pane, Rectangle shape)
   {
      super(true);
      this.pane = pane;
      this.shape = shape;
      this.scheduleAtFixedRate(new Task(), 0, 10);
   }

   private static int boolToInt(boolean bool)
   {
      return bool ? 1 : -1;
   }

   private final class Task extends TimerTask
   {
      @Override
      public void run()
      {
         shape.setLayoutX(shape.getLayoutX() + boolToInt(moveRight));
         shape.setLayoutY(shape.getLayoutY() + boolToInt(moveBottom));

         if (shape.getLayoutX() >= pane.getWidth() - shape.getWidth())
         {
            moveRight = false; 
         }
         if (shape.getLayoutY() >= pane.getHeight() - shape.getHeight())
         {
            moveBottom = false;
         }
         if (shape.getLayoutX() <= 0)
         {
            moveRight = true;
         }
         if (shape.getLayoutY() <= 0)
         {
            moveBottom = true;
         }
      }
   }
}  

JFXTester.java (main class)

import javafx.application.Application;
import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.StackPane;
import javafx.scene.paint.Color;
import javafx.scene.shape.Rectangle;
import javafx.stage.Stage;

public class JFXTester extends Application
{

   StackPane root;

   @Override
   public void start(Stage primaryStage)
   {

      root = new StackPane();
      root.getChildren().add(createStartButton());

      Scene scene = new Scene(root, 300, 250);

      primaryStage.setTitle("Hello World!");
      primaryStage.setScene(scene);
      primaryStage.show();
   }

   private Rectangle createRectangle()
   {
      Rectangle r = new Rectangle();
      r.setX(0);
      r.setY(0);
      r.setWidth(30);
      r.setHeight(30);
      r.setFill(new Color(0.8, 0.7, 0.6, 0.5));

      return r;
   }

   private Button createStartButton()
   {
      final Button btn = new Button();
      btn.setText("START!");
      btn.setOnAction(new EventHandler<ActionEvent>()
      {
         @Override
         public void handle(ActionEvent event)
         {
            btn.setVisible(false);
            Rectangle r = createRectangle();
            root.getChildren().add(r);
            new RectangleMover(root, r);
         }
      });
      return btn;
   }

   public static void main(String[] args)
   {
      launch(args);
   }
}  

How to run application (java 1.7.0_51-b13):

D:\.temporary\java>dir /a-d /b /s
D:\.temporary\java\jfxtester\JFXTester.java
D:\.temporary\java\jfxtester\RectangleMover.java

D:\.temporary\java>%JAVA_HOME%\bin\javac -cp .;%JRE_HOME_7%\lib\jfxrt.jar ./jfxtester/JFXTester.java    

D:\.temporary\java>%JRE_HOME_7%\bin\java -cp .;%JRE_HOME_7%\lib\jfxrt.jar jfxtester.JFXTester

Answer 1

No, this is not the correct way to do it. I would recommend that you read JavaFX Transitions documentation.

I personally could not get your code to work properly (Using Java 8, but that should not matter as Java is always backwards compatible). The rectangle showed on the screen, but it did not move at all.

I got this exception:

Exception in thread "Timer-0" java.lang.IllegalStateException: Not on FX application thread; currentThread = Timer-0
at com.sun.javafx.tk.Toolkit.checkFxUserThread(Unknown Source)
at com.sun.javafx.tk.quantum.QuantumToolkit.checkFxUserThread(Unknown Source)
at javafx.scene.Scene.addToDirtyList(Unknown Source)
at javafx.scene.Node.addToSceneDirtyList(Unknown Source)
at javafx.scene.Node.impl_markDirty(Unknown Source)
at javafx.scene.shape.Shape.impl_markDirty(Unknown Source)
at javafx.scene.Node.impl_transformsChanged(Unknown Source)
at javafx.scene.Node$13.invalidated(Unknown Source)
at javafx.beans.property.DoublePropertyBase.markInvalid(Unknown Source)
at javafx.beans.property.DoublePropertyBase.set(Unknown Source)
at javafx.scene.Node.setLayoutX(Unknown Source)

One I fixed that problem to make it run on the JavaFX thread, the rectangle still did not move an inch pixel.

About the cleaniness of your code:

public class RectangleMover extends Timer

extends Timer is a very bad smell here. There's absolutely no reason to extend Timer. Use a timer as a field inside your class instead. Prefer composition over inheritance.

Speaking of Timer, you could say that it is more or less deprecated, it is often better to use a ScheduledExecutorService. However, if you do the animation properly, you will not need a Timer or ExecutorService at all.

I do like how you're using your boolToInt method (just a shame that I can't the results of it does not seem to be working). I also like how you have separated your methods overall.

I just don't like that your code does not seem to be working for me...