JavaFX controller for a poker game

Question

I am making a poker application in JavaFX. When I try to display the cards on the screen it is working perfectly fine. Yet I have a method with a lot of duplicated code in it.

I have tried to compress the code into 1 function with a for loop but I couldn't get it to work. Anyone has an idea how to get rid of the duplication?

public class GameController implements Initializable {
 @FXML
 private Canvas canvasBoard;
 @FXML
 private Canvas canvasPlayer1;
 @FXML
 private Canvas canvasPlayer2;
 @FXML
 private Canvas canvasPlayer3;
 @FXML
 private Canvas canvasPlayer4;
 private int playerCardsWidth1 = 0;
 private int playerCardsWidth2 = 0;
 private int playerCardsWidth3 = 0;
 private int playerCardsWidth4 = 0;
 private int dealerCardsWidth = 0;
 private TexasHoldem game;
 private IPlayer board;
 public GameController() {
 try {
 game = new TexasHoldem();
 } catch (Exception e) {
 e.printStackTrace();
 }
 }
 private void showCardsBoard() {
 board = game.getBoard();
 dealerCardsWidth += 50;
 for (ICard c : board.getCards()) {
 String foto = c.getFilename();
 Image image = new Image(foto);
 GraphicsContext gcBoard = canvasBoard.getGraphicsContext2D();
 int dealerCardsHeight = 0;
 gcBoard.drawImage(image,dealerCardsWidth, dealerCardsHeight,60,80);
 dealerCardsWidth += 75;
 }
 }
 private void showCardsPlayers() {
 List<IPlayer> spelers = game.getPlayers();
 for (int i = 0; i < spelers.size(); i++) {
 if (i == 0) {
 IPlayer player1 = spelers.get(i);
 for (ICard c : player1.getCards()) {
 Image image = new Image(c.getFilename());
 GraphicsContext gc1 = canvasPlayer1.getGraphicsContext2D();
 int playerCardsHeight1 = 0;
 gc1.drawImage(image, playerCardsWidth1, playerCardsHeight1, 60, 80);
 playerCardsWidth1 += 75;
 }
 }
 if (i == 1) {
 IPlayer player2 = spelers.get(i);
 for (ICard c : player2.getCards()) {
 Image image = new Image(c.getFilename());
 GraphicsContext gc2 = canvasPlayer2.getGraphicsContext2D();
 int playerCardsHeight2 = 0;
 gc2.drawImage(image, playerCardsWidth2, playerCardsHeight2, 60, 80);
 playerCardsWidth2 += 75;
 }
 }
 if (i == 2) {
 IPlayer player3 = spelers.get(i);
 for (ICard c : player3.getCards()) {
 Image image = new Image(c.getFilename());
 GraphicsContext gc3 = canvasPlayer3.getGraphicsContext2D();
 int playerCardsHeight3 = 0;
 gc3.drawImage(image, playerCardsWidth3, playerCardsHeight3, 60, 80);
 playerCardsWidth3 += 75;
 }
 }
 if (i == 3) {
 IPlayer player4 = spelers.get(i);
 for (ICard c : player4.getCards()) {
 Image image = new Image(c.getFilename());
 GraphicsContext gc4 = canvasPlayer4.getGraphicsContext2D();
 int playerCardsHeight4 = 0;
 gc4.drawImage(image, playerCardsWidth4, playerCardsHeight4, 60, 80);
 playerCardsWidth4 += 75;
 }
 }
 } 
 }

Answer 1

Logic:

If statements differs only in 2 variables: canvasPlayerX and playerCardsWidthX.

playerCardsWidthX: looks like this var can be converted to local, just like playerCardsHeightX;

canvasPlayerX: you can do one little trick here - add player canvases to a list in initialize() method, and then refer to the i-th player canvas as playersCanvases.get(i);

Moreover, your code didnt even compile, because GameController should implement initialize(..) method;

Codestyle:

• playerCardsWidthX - in your code, its not width of player cards, its coordinate, where to draw next card. Consider renaming;

• in constructor, exception is caught, and stacktrace is printed, but after that, program will continue its flow. Is this what you want? (see code below for possible solution).

• extract constants into private static final fields

---

public class GameController implements Initializable {
public static final int CARD_WIDTH = 60;
public static final int CARD_HEIGHT = 80;
public static final int CARD_SPACING = 15;
public static final int CARDS_X_DIFF = CARD_WIDTH + CARD_SPACING;
@FXML
private Canvas canvasBoard;
@FXML
private Canvas canvasPlayer1;
@FXML
private Canvas canvasPlayer2;
@FXML
private Canvas canvasPlayer3;
@FXML
private Canvas canvasPlayer4;
private List<Canvas> playersCanvases;
private TexasHoldem game;
private IPlayer board;
public GameController() {
 try {
 game = new TexasHoldem();
 } catch (Exception e) {
 e.printStackTrace();
 Platform.exit();
 }
}
@Override
public void initialize(URL url, ResourceBundle resourceBundle) {
 // in initialize method, because in constructor canvases are not assigned(== null)
 playersCanvases = Arrays.asList(canvasPlayer1, canvasPlayer2, canvasPlayer3, canvasPlayer4);
}
private void showBoardCards() {
 board = game.getBoard();
 showCards(50, board.getCards(), canvasBoard.getGraphicsContext2D());
}
private void showPlayersCards() {
 List<IPlayer> players = game.getPlayers();
 for (int i = 0; i < players.size(); i++) {
 IPlayer player = players.get(i);
 showCards(0, player.getCards(), playersCanvases.get(i).getGraphicsContext2D());
 }
}
private void showCards(int xOffset, List<ICard> cards, GraphicsContext gc) {
 int cardY = 0;
 int cardX = xOffset;
 for (ICard c : cards) {
 Image image = new Image(c.getFilename());
 gc.drawImage(image, cardX, cardY, CARD_WIDTH, CARD_HEIGHT);
 cardX += CARDS_X_DIFF;
 }
}

}