Most of my programming experience is with Objective-C, but I have recently started learning Java and JavaFX to build the user interface for a Trading Card Game.  This is the most Java I have ever written, and I am completely new to JavaFX as well, so I am sure that there are lots of problems with this code.  I mostly used Scene Builder to layout the scene itself, but I manually edited the FXMLDocument file also to accomplish everything.  I am not at all sure that I am doing things the right way here.  I have tried to make the code as readable as possible, but many objects are created programmatically and not with Scene Builder so I am not sure that everything will make sense to the reader.  I should note that I tried to use only Panes for containing the information and for rendering it, but I had issues with the scene not updating properly.  To solve this problem, I created Lists that contain the Groups that the scene will actually render.  This separates the rendering from the creation of the objects being rendered.  I think this is a good approach, but I may be wrong.What I am looking for are best practices for Java, as well as any Java features I may not be using that I should be. Also the readability of the code is always very important to me.  Don't worry about being harsh or critical, you won't hurt my feelings.Here is a sample image of the GUI in action. I should note that it is a prototype for the purpose of testing the game as quickly as possible, so it is ugly:First things first, this is the class that is the launching point for the program.JavaFXGame.javapackage com.cardshifter.fx;import javafx.application.Application;import javafx.fxml.FXMLLoader;import javafx.scene.Parent;import javafx.scene.Scene;import javafx.stage.Stage;//This class just loads the FXML document which initializes its DocumentControllerpublic class JavaFXGame extends Application {    @Override    public void start(Stage stage) throws Exception {             //stage.setTitle(title);        Parent root = FXMLLoader.load(getClass().getResource(FXMLDocument.fxml));        Scene scene = new Scene(root);        stage.setScene(scene);        //stage.centerOnScreen();        stage.show();    }     // @param args the command line arguments    public static void main(String[] args) {        launch(args);    }}This is the meat of the program, where the user interface is controlled and commands are sent to the Game:FXMLGameController.javapackage com.cardshifter.fx;import java.io.File;import java.io.FileInputStream;import java.io.InputStream;import java.net.URL;import java.util.ArrayList;import java.util.List;import java.util.ResourceBundle;import javafx.event.ActionEvent; import javafx.fxml.FXML;import javafx.fxml.Initializable;import javafx.scene.Group;import javafx.scene.Node;import javafx.scene.control.Label;import javafx.scene.layout.Pane;import javafx.scene.paint.Color;import javafx.scene.shape.QuadCurve;import javafx.scene.shape.Rectangle;import org.luaj.vm2.lib.jse.CoerceLuaToJava;import com.cardshifter.ai.CardshifterAI;import com.cardshifter.ai.CompleteIdiot;import com.cardshifter.core.Card;import com.cardshifter.core.Game;import com.cardshifter.core.Player;import com.cardshifter.core.Targetable;import com.cardshifter.core.Zone;import com.cardshifter.core.actions.TargetAction;import com.cardshifter.core.actions.UsableAction;import com.cardshifter.core.console.CommandLineOptions;public class FXMLGameController implements Initializable {    //INITIAL GAME SETUP    private final CardshifterAI opponent = new CompleteIdiot();    //need a forward declaration so that this is  global to the class    private Game game;    //hack to make the buttons work properly    private boolean gameHasStarted = false;    //I think this is a public constructor, this code initializes the Game    @FXML    Pane anchorPane;    public FXMLGameController() throws Exception {        this.initializeGame();    }    private void initializeGame() throws Exception {        CommandLineOptions options = new CommandLineOptions();        InputStream file = options.getScript() == null ? Game.class.getResourceAsStream(start.lua) : new FileInputStream(new File(options.getScript()));        game = new Game(file, options.getRandom());    }    //GAME START and NEW GAME    @FXML    private Label startGameLabel;    @FXML    private void startGameButtonAction(ActionEvent event) {        if (gameHasStarted == false) {            startGameLabel.setText(Starting Game);            game.getEvents().startGame(game);            gameHasStarted = true;            turnLabel.setText(String.format(Turn Number %d, game.getTurnNumber()));            this.playerBattlefieldData = new ArrayList<>();            this.opponentBattlefieldData = new ArrayList<>();            this.playerHandData = new ArrayList<>();            this.opponentHandData = new ArrayList<>();            this.createData();            this.render();        }    }    @FXML    private void newGameButtonAction(ActionEvent event) throws Exception {        this.initializeGame();        startGameLabel.setText(Starting Game);        game.getEvents().startGame(game);        gameHasStarted = true;        turnLabel.setText(String.format(Turn Number %d, game.getTurnNumber()));        this.playerBattlefieldData = new ArrayList<>();        this.opponentBattlefieldData = new ArrayList<>();        this.playerHandData = new ArrayList<>();        this.opponentHandData = new ArrayList<>();        this.createData();        this.render();    }    //TODO: Create a fixed time step and render in that    //RENDER - eventually change this to an Update method    public void render() {        //this is a hack to make the buttons disappear when the choice is made        //will not work with a fixed time step        Node choiceBoxPane = anchorPane.lookup(#choiceBoxPane);        anchorPane.getChildren().remove(choiceBoxPane);        this.renderHands();        this.renderBattlefields();        this.updateGameLabels();    }    //RENDER HANDS    @FXML    private Pane opponentHandPane;    @FXML    private Pane playerHandPane;    private void renderHands() {        this.renderOpponentHand();        this.renderPlayerHand();    }    private void renderOpponentHand() {        opponentHandPane.getChildren().clear();        opponentHandPane.getChildren().addAll(opponentHandData);    }    private void renderPlayerHand() {        playerHandPane.getChildren().clear();        playerHandPane.getChildren().addAll(playerHandData);    }    //RENDER BATTLEFIELDS    @FXML    Pane opponentBattlefieldPane;    @FXML    Pane playerBattlefieldPane;    private void renderBattlefields() {        this.renderOpponentBattlefield();        this.renderPlayerBattlefield();    }    private void renderOpponentBattlefield() {        opponentBattlefieldPane.getChildren().clear();        opponentBattlefieldPane.getChildren().addAll(opponentBattlefieldData);    }    private void renderPlayerBattlefield() {        playerBattlefieldPane.getChildren().clear();        playerBattlefieldPane.getChildren().addAll(playerBattlefieldData);    }    //Called at the end of turn, and when a card is played    public void createData() {        this.createHands();        this.createBattlefields();    }    //CREATE HANDS    private void createHands() {        this.createOpponentHand();        this.createPlayerHand();    }    //CREATE OPPONENT CARD BACKS    private List<Group> opponentHandData;    private void createOpponentHand() {        opponentHandData.clear();        //Opponent cards are rendered differently because the faces are not visible        double paneHeight = opponentHandPane.getHeight();        double paneWidth = opponentHandPane.getWidth();        int numCards = this.getOpponentCardCount();        int maxCards = Math.max(numCards, 8);        double cardWidth = paneWidth / maxCards;        int currentCard = 0;        while(currentCard < numCards) {            Group cardGroup = new Group();            cardGroup.setTranslateX(currentCard * (cardWidth*1.05)); //1.05 so there is space between cards            opponentHandData.add(cardGroup);            Rectangle cardBack = new Rectangle(0,0,cardWidth,paneHeight);            cardBack.setFill(Color.AQUAMARINE);            cardGroup.getChildren().add(cardBack);            currentCard++;        }    }    private int getOpponentCardCount() {        Player player = game.getLastPlayer();         Zone hand = (Zone)CoerceLuaToJava.coerce(player.data.get(hand), Zone.class);        List<Card> cardsInHand = hand.getCards();        return cardsInHand.size();    }    //CREATE PLAYER HAND    private List<Group> playerHandData;    private void createPlayerHand() {        playerHandData.clear();        List<Card> cardsInHand = this.getCurrentPlayerHand();        int numCards = cardsInHand.size();        int cardIndex = 0;        for (Card card : cardsInHand) {            CardNode cardNode = new CardNode(playerHandPane, numCards, testName, card, this);            Group cardGroup = cardNode.getCardGroup();            cardGroup.setAutoSizeChildren(true); //NEW            cardGroup.setId(String.format(card%d, card.getId()));            cardGroup.setTranslateX(cardIndex * cardNode.getWidth());            playerHandData.add(cardGroup);            cardIndex++;        }    }    private List<Card> getCurrentPlayerHand() {        Player player = game.getFirstPlayer();         Zone hand = (Zone)CoerceLuaToJava.coerce(player.data.get(hand), Zone.class);        return hand.getCards();    }    //CREATE BATTLEFIELDS    private void createBattlefields() {        this.createOpponentBattlefield();        this.createPlayerBattlefield();    }    //CREATE OPPONENT BATTLEFIELD    private List<Group> opponentBattlefieldData;    private void createOpponentBattlefield() {        opponentBattlefieldData.clear();        List<Card> cardsInBattlefield = this.getBattlefield(game.getLastPlayer());        int numCards = cardsInBattlefield.size();        int cardIndex = 0;        for (Card card : cardsInBattlefield) {            CardNodeBattlefield cardNode = new CardNodeBattlefield(opponentBattlefieldPane, numCards, testName, card, this, false);            Group cardGroup = cardNode.getCardGroup();            cardGroup.setAutoSizeChildren(true); //NEW            cardGroup.setId(String.format(card%d, card.getId()));            cardGroup.setTranslateX(cardIndex * cardNode.getWidth());            opponentBattlefieldData.add(cardGroup);            cardIndex++;        }     }    //CREATE PLAYER BATTLEFIELD    private List<Group> playerBattlefieldData;    private void createPlayerBattlefield() {        playerBattlefieldData.clear();        List<Card> cardsInBattlefield = this.getBattlefield(game.getFirstPlayer());        int numCards = cardsInBattlefield.size();        int cardIndex = 0;        for (Card card : cardsInBattlefield) {            CardNodeBattlefield cardNode = new CardNodeBattlefield(playerBattlefieldPane, numCards, testName, card, this, true);            Group cardGroup = cardNode.getCardGroup();            cardGroup.setAutoSizeChildren(true); //NEW            cardGroup.setId(String.format(card%d, card.getId()));            cardGroup.setTranslateX(cardIndex * cardNode.getWidth());            playerBattlefieldData.add(cardGroup);            cardIndex++;        }     }    private List<Card> getBattlefield(Player player) {        Zone battlefield = (Zone)CoerceLuaToJava.coerce(player.data.get(battlefield), Zone.class);        return battlefield.getCards();    }    //END TURN LABEL    @FXML    private Label turnLabel;    //ADVANCE TURNS    @FXML    private void handleTurnButtonAction(ActionEvent event) {        if (gameHasStarted == true) {            game.nextTurn();            //This is the AI doing the turn action            while (game.getCurrentPlayer() == game.getLastPlayer()) {                UsableAction action = opponent.getAction(game.getCurrentPlayer());                if (action == null) {                    System.out.println(Warning: Opponent did not properly end turn);                    break;                }                action.perform();            }            turnLabel.setText(String.format(Turn Number %d, game.getTurnNumber()));            //reload data at the start of a new turn            this.createData();            this.render();        }    }    //CHOICE BOX PANE    public void buildChoiceBoxPane(Card card, List<UsableAction> actionList) {        Pane choiceBoxPane = new Pane();        choiceBoxPane.setPrefHeight(367);        choiceBoxPane.setPrefWidth(550);        choiceBoxPane.setTranslateX(326);        choiceBoxPane.setTranslateY(150);        choiceBoxPane.setId(choiceBoxPane);        choiceBoxPane.getChildren().clear();        int numChoices  = actionList.size();        double paneHeight = choiceBoxPane.getPrefHeight();        double paneWidth = choiceBoxPane.getPrefWidth();        double choiceBoxWidth = paneWidth / numChoices;        int actionIndex = 0;        for(UsableAction action : actionList) {            ChoiceBoxNode choiceBox = new ChoiceBoxNode(choiceBoxWidth, paneHeight, testName, action, this);            Group choiceBoxGroup = choiceBox.getChoiceBoxGroup();            choiceBoxGroup.setTranslateX(actionIndex * choiceBoxWidth);            choiceBoxPane.getChildren().add(choiceBox.getChoiceBoxGroup());            actionIndex++;        }        anchorPane.getChildren().add(choiceBoxPane);    }    //TARGETING    public TargetAction nextAction;    public void performNextAction(Targetable target) {        nextAction.setTarget(target);        nextAction.perform();        this.createData();        this.render();    }    public void markTargets(List<Card> targets) {        List<Node> cardsInPlayerBattlefield = playerBattlefieldPane.getChildren();        List<Node> cardsInOpponentBattlefield = opponentBattlefieldPane.getChildren();        for (Card target : targets) {            for(Node node : cardsInPlayerBattlefield) {                if(node.getId().equals(String.format(card%d,target.getId())) == true) {                    CardNodeBattlefield actionNode = (CardNodeBattlefield)node;                    actionNode.createTargetButton();                }            }            for(Node node : cardsInOpponentBattlefield) {                if(node.getId().equals(String.format(card%d,target.getId())) == true) {                    CardNodeBattlefield actionNode = (CardNodeBattlefield)node;                    actionNode.createTargetButton();                }            }        }    }    //GAME STATE LABELS    @FXML    Label opponentLife;    @FXML    Label opponentCurrentMana;    @FXML    Label opponentTotalMana;    @FXML    Label opponentScrap;    @FXML    Label playerLife;    @FXML    Label playerCurrentMana;    @FXML    Label playerTotalMana;    @FXML    Label playerScrap;    private void updateGameLabels() {        Player opponent = game.getLastPlayer();        opponentLife.setText(String.format(%d,opponent.data.get(life).toint()));        opponentCurrentMana.setText(String.format(%d,opponent.data.get(mana).toint()));        opponentTotalMana.setText(String.format(%d,opponent.data.get(manaMax).toint()));        opponentScrap.setText(String.format(%d,opponent.data.get(scrap).toint()));        Player player = game.getFirstPlayer();        playerLife.setText(String.format(%d, player.data.get(life).toint()));        playerCurrentMana.setText(String.format(%d, player.data.get(mana).toint()));        playerTotalMana.setText(String.format(%d, player.data.get(manaMax).toint()));        playerScrap.setText(String.format(%d, player.data.get(scrap).toint()));    }    //BOILERPLATE    @Override    public void initialize(URL url, ResourceBundle rb) {        // TODO    }           //NOT YET USED    @FXML    private QuadCurve handGuide;}Finally, this is the FXML file that controls the layout for all the objects that are not created programmatically:FXMLDocument.fxml<?xml version=1.0 encoding=UTF-8?><?import javafx.scene.text.*?><?import javafx.scene.shape.*?><?import java.lang.*?><?import java.util.*?><?import javafx.scene.*?><?import javafx.scene.control.*?><?import javafx.scene.layout.*?><AnchorPane fx:id=anchorPane maxHeight=600 maxWidth=1200 minHeight=600 minWidth=1200 prefHeight=600.0 prefWidth=1200.0 xmlns=http://javafx.com/javafx/8 xmlns:fx=http://javafx.com/fxml/1 fx:controller=com.cardshifter.fx.FXMLGameController>    <children>        <!--  Buttons  -->        <Button fx:id=button layoutX=1098.0 layoutY=530.0 onAction=#startGameButtonAction text=Start Game />        <Label fx:id=startGameLabel layoutX=1098.0 layoutY=564.0 minHeight=16 minWidth=69 />        <Button layoutX=1105.0 layoutY=467.0 mnemonicParsing=false onAction=#handleTurnButtonAction text=End Turn />        <Label fx:id=turnLabel layoutX=1091.0 layoutY=500.0 text=Turn Number />        <Button fx:id=newGameButton layoutX=17.0 layoutY=24.0 mnemonicParsing=false onAction=#newGameButtonAction text=New Game />        <!--  Hands  -->        <Pane fx:id=playerHandPane layoutX=14.0 layoutY=393.0 prefHeight=200.0 prefWidth=1066.0>            <children>                <QuadCurve fx:id=handGuide controlY=-100.0 endX=100.0 fill=DODGERBLUE layoutX=480.0 layoutY=200.0 opacity=0.0 startX=-100.0 stroke=BLACK strokeType=INSIDE />            </children>        </Pane>        <Pane fx:id=opponentHandPane layoutX=153.0 layoutY=6.0 prefHeight=61.0 prefWidth=895.0 />        <!--  Battlefields  -->        <Pane fx:id=playerBattlefieldPane layoutX=47.0 layoutY=237.0 prefHeight=98.0 prefWidth=1001.0 />        <Pane fx:id=opponentBattlefieldPane layoutX=47.0 layoutY=107.0 prefHeight=98.0 prefWidth=1001.0 />        <!--  Game State Labels  -->        <Label layoutX=1111.0 layoutY=29.0 text=Opponent />        <Label layoutX=1133.0 layoutY=51.0 text=Life />        <Label layoutX=1101.0 layoutY=91.0 text=Current Mana />        <Label layoutX=1107.0 layoutY=131.0 text=Total Mana />        <Label fx:id=opponentLife layoutX=1133.0 layoutY=67.0 prefHeight=16.0 prefWidth=35.0 text=# textAlignment=CENTER />        <Label fx:id=opponentCurrentMana layoutX=1140.0 layoutY=107.0 text=# />        <Label fx:id=opponentTotalMana layoutX=1140.0 layoutY=148.0 text=# />        <Label layoutX=1126.0 layoutY=263.0 text=Player />        <Label layoutX=1133.0 layoutY=292.0 text=Life />        <Label fx:id=playerLife layoutX=1138.0 layoutY=308.0 text=# />        <Label layoutX=1103.0 layoutY=327.0 text=Current Mana />        <Label fx:id=playerCurrentMana layoutX=1140.0 layoutY=343.0 text=# />        <Label layoutX=1111.0 layoutY=366.0 text=Total Mana />        <Label fx:id=playerTotalMana layoutX=1141.0 layoutY=385.0 text=# />        <Label layoutX=1127.0 layoutY=171.0 text=Scrap />        <Label layoutX=1129.0 layoutY=407.0 text=Scrap />        <Label fx:id=playerScrap layoutX=1141.0 layoutY=423.0 text=# />        <Label fx:id=opponentScrap layoutX=1140.0 layoutY=187.0 text=# />    </children></AnchorPane>
Trading Card Game Prototype GUI with JavaFX
java;game;gui;community challenge;javafx
Your JavaFXGame class can use javadoc/*** This class just loads the FXML document which initializes its DocumentController*/public class JavaFXGame extends Application {The start method can be shortened and cleaned up:@Overridepublic void start(Stage stage) throws Exception {         Parent root = FXMLLoader.load(getClass().getResource(FXMLDocument.fxml));    Scene scene = new Scene(root);    stage.setScene(scene);    stage.show();}Even though the specification from the Application class specifies that throws Exception, you can declare it as the following:public void start(Stage stage) throws IOException {     Specifying the exception more is allowed in an overridden method, it is also allowed to get rid of it completely (although that's not necessary here as that would require a try-catch inside the method). This doesn't break the contract.You have a comment for main which looks a bit like Javadoc, but please use real javadoc instead:/** * Main method * @param args the command line arguments */public static void main(String[] args) {    launch(args);}Taking a look at just the first part of your other class and we can see that it is filled with unnecessary comments and other fluff://INITIAL GAME SETUPprivate final CardshifterAI opponent = new CompleteIdiot();//need a forward declaration so that this is  global to the classprivate Game game;//hack to make the buttons work properlyprivate boolean gameHasStarted = false;//I think this is a public constructor, this code initializes the Game@FXMLPane anchorPane;public FXMLGameController() throws Exception {    this.initializeGame();}You have several unnecessary comments here (although I understand some of them are because you are new to the language).private final CardshifterAI opponent = new CompleteIdiot();private Game game;private boolean gameHasStarted = false;@FXMLPane anchorPane;public FXMLGameController() throws Exception {    this.initializeGame();}You seem to be very inconsistent in using private on the fields. Remember to use private whenever possible.It is standard practice in Java to keep all class-variables (a.k.a. fields) at the top of the class together. This makes a typical Java class have the order 1. fields 2. constructor 3. methods. 4. static-methods (where you place the static-methods can differ, but they should always be put together whenever there are any).On some occasions you seem to be doing this:    if (gameHasStarted == false) {        // a lot of code...    }    // end of methodIn such cases I personally prefer to return early to reduce indentation.Also, you can use !gameHasStarted instead of comparing with == false. ! is the logical not-operator in Java.    if (!gameHasStarted) {        return;    }    // a lot of code...This also applies to the handleTurnButtonAction method, but there you can simply remove == true as comparing with true is the same as returning the boolean directly.startGameButtonAction and newGameButtonAction are way too similar to be two separate methods. Consider calling one from the other. Also ask yourself this question: Is startGame button really needed? And/Or is the call to initializeGame in the constructor really needed? Consider making it start a new game automatically on startup.Either way, there's code duplication there that is easy to get rid of. (Extract function is one way, but as said, consider changing the user experience)The Java conventions for spacing is like this:    while (currentCard < numCards) {        ...        cardGroup.setTranslateX(currentCard * cardWidth * 1.05); // note that a set of parenthesis can be removed here        Rectangle cardBack = new Rectangle(0, 0, cardWidth, paneHeight);createOpponentBattlefield, createPlayerBattlefield and createPlayerHand is way too similar. As the game model seems to use a Zone class, perhaps you should make a ZoneView class that is responsible for displaying a Zone.There is also some duplicate code in markTargets, but I believe that can be handled by using a ZoneView class. Then you could call markTargets on each ZoneView.Finally, I personally prefer a little bit more space when there is a whole bunch of annotations and fields together. Such as the following:@FXMLLabel opponentCurrentMana;@FXMLLabel opponentTotalMana;@FXMLLabel opponentScrap;@FXMLLabel playerLife;@FXMLLabel playerCurrentMana;Alternatively, you can write the annotation on the same line as the field type and name.@FXML Label opponentCurrentMana;@FXML Label opponentTotalMana;@FXML Label opponentScrap;@FXML Label playerLife;@FXML Label playerCurrentMana;P.S. Don't forget private.