1
\$\begingroup\$

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;
                }
            }
        }      
    }
\$\endgroup\$
0
\$\begingroup\$

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;
    }
}

}

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.