Taking flight with JavaFX

Question

Just another step in my educational Trek. I didn't know when/why I would use the transparent feature and in experimenting with it, I made this.

This one was fun to behold once done. It's an animated flying gif and you can move it around the screen and press keys to alter its speed or add effects to the image itself.

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.Group;
import javafx.scene.effect.Bloom;
import javafx.scene.effect.Glow;
import javafx.scene.effect.MotionBlur;
import javafx.scene.effect.Reflection;
import javafx.scene.effect.Shadow;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.input.KeyCode;
import javafx.scene.paint.Color;
import javafx.stage.Stage;
import javafx.stage.StageStyle;
public class AnimatedFun extends Application {
 int animationSpeed = 5;
 public static void main(String[] args) {
 launch(args);
 }
 @Override
 public void start(Stage stage) {
 ImageView view = new ImageView(new Image("https://d2aao99y1mip6n.cloudfront.net/images/runes/idols/eJ8HJ8Ge9Ae9Be9Ae9Bj9Fj9Geigtzillnuhymn.gif"));
 view.setEffect(new Glow());
 view.setOnMouseClicked(e -> stage.close());
 view.setFocusTraversable(true);
 view.setOnKeyPressed(e -> { 
 if (e.getCode() == KeyCode.ESCAPE) { 
 stage.close();
 } else if (e.getCode() == KeyCode.SPACE) {
 view.setEffect(new Reflection());
 } else if (e.getCode() == KeyCode.DELETE) {
 view.setEffect(null);
 } else if (e.getCode() == KeyCode.ENTER) {
 view.setEffect(new Bloom());
 } else if (e.getCode() == KeyCode.SHIFT) {
 view.setEffect(new Shadow());
 } else if (e.getCode() == KeyCode.RIGHT) {
 stage.setX(stage.getX() + animationSpeed);
 } else if (e.getCode() == KeyCode.LEFT) {
 stage.setX(stage.getX() - animationSpeed);
 } else if (e.getCode() == KeyCode.UP) {
 stage.setY(stage.getY() - animationSpeed);
 } else if (e.getCode() == KeyCode.DOWN) {
 stage.setY(stage.getY() + animationSpeed);
 } else if (e.getCode() == KeyCode.CONTROL) {
 animationSpeed += 3;
 if (animationSpeed >= 15) {
 view.setEffect(new MotionBlur());
 }
 } else if (e.getCode() == KeyCode.ALT) {
 if (animationSpeed >= 15 && animationSpeed < 18){
 view.setEffect(new Glow());
 }
 animationSpeed = (Math.max(5, animationSpeed - 2));
 }
 });
 Group root = new Group();
 root.getChildren().add(view);
 stage.setScene(new Scene(root, Color.TRANSPARENT));
 stage.initStyle(StageStyle.TRANSPARENT);
 stage.getIcons().add(new Image("https://d2aao99y1mip6n.cloudfront.net/images/runes/lg/eJ8HJ8Ge9Ae9Be9Ae9Bj9Fj9Geigtzillnuhymn.jpg"));
 stage.show();
 }
}

Feel free download the runnable jar.

I'd like a general review of its current state as well as any befitting improvements, be it concerning JavaFX convention, or just the general structure.

Pardon the horizontally unkempt-ness, side-effect of porting over a usable version using online links for the images.

• Phrancis: Nice title! I love this.

Answer 1

It would be better to move all the numbers that appear in this fragment into descriptively named constant fields:

 } else if (e.getCode() == KeyCode.CONTROL) {
 animationSpeed += 3;
 if (animationSpeed >= 15) {
 view.setEffect(new MotionBlur());
 }
 } else if (e.getCode() == KeyCode.ALT) {
 if (animationSpeed >= 15 && animationSpeed < 18){
 view.setEffect(new Glow());
 }
 animationSpeed = (Math.max(5, animationSpeed - 2));
 }

If you give these numbers names, the code will become more intuitive. It will also reduce errors in changing values that would have to updated at multiple places, for example the number 15 in animationSpeed >= 15.

If you put the constants at the top of the class, it will be easier to tweak the interesting parameters in one place, rather than rummaging through the code.

Likewise, it will be good to put the image urls in constants too. You have two different links, but it's not obvious what's the difference. By putting these values into named constants, it will become clear. And the lines should become slightly less wide.

Answer 2

I'd pull out that huge if/else block into a switch block.

This will make your code more readable. (Also slightly better performance.) Right now it's difficult to see the logic because it is hidden behind code duplication. See how a switch block makes it very easy to see the possible keycodes and their corresponding behaviors.

switch(e.getCode()) {
 case(KeyCode.ESCAPE) : stage.close(); break;
 case(KeyCode.DELETE) : view.setEffect(null); break;
 case(KeyCode.SPACE) : view.setEffect(new Reflection()); break;
 ...