1
\$\begingroup\$

I am trying to improve my coding skills in Java by taking existing code, studying it, and adding on to it. In this code example, I took code from Connect Four game in Java and edited it. The most major edit I created for the game was that I created a GUI for this game. From looking at the code, are there any suggestions on how I can improve my coding ability in Java?

import java.util.stream.Collectors; 
import java.util.stream.IntStream; 
import java.awt.event.*;
import javax.swing.*;
import java.util.*;
/**This program uses GUI, including a JTextField. Both javax.swing.* and java.awt.event.* need to be imported in this program.
 *This program also uses java.util.*
 */
public class ConnectFour {
    private static final char[] players = new char[] { 'X', 'O' };

    private final int width, height;

    private int lastCol = -1, lastTop = -1;

    private GUI gui=new GUI(); //The GUI class needs to be used in the ConnectFour class to have GUI for this game.

    final char[][] grid;


    public ConnectFour(int width, int height) {
        this.width = width;
        this.height = height;
        this.grid = new char[height][];
        for (int h = 0; h < height; h++) {
            Arrays.fill(this.grid[h] = new char[width], '.'); 
        }
    }

    public String toString() { 
        return IntStream.range(0, this.width)
                        .mapToObj(Integer::toString)
                        .collect(Collectors.joining()) + "\n" +
               Arrays.stream(this.grid)
                     .map(String::new)
                     .collect(Collectors.joining("\n"));
    }

    /**
     * Prompts the user for a column, repeating until a valid
     * choice is made.
     */
    public void chooseAndDrop(char symbol) {
        //This string variable is used by a label to give details on whether a user entered a valid column number or not.
        String columnInfo=""; 
        do {
            /**The chooseAndDrop GUI will update two labels to display important information to the user(s).
             * It will tell the user(s) whether Player X or Player O is supposed to play and also if a
             * valid column number was entered by the users.
             */
            gui.chooseAndDropGUI(symbol, columnInfo); 
            int col = gui.readPlayerInputForProcessing();
            columnInfo="";
            if (! (0 <= col && col < this.width)) {
                /**If a user entered a column number that is outside of the accepted range,
                 * the columnInfo variable is updated to tell the user to pick a column
                 * in the accepted range.
                 */
                columnInfo="Column must be between 0 and " +
                        (this.width - 1); 
                continue;
            }
            for (int h = this.height - 1; h >= 0; h--) {
                if (this.grid[h][col] == '.') {
                    this.grid[this.lastTop=h][this.lastCol=col] = symbol;
                    return;
                }
            }
            /**If a user entered in a column number and that column is already full,
             * the columnInfo variable is updated to tell the user to pick a different column.
             */
            columnInfo="Column "+col+" is full.";

        }while (true);
}

    /**
     * Detects whether the last chip played was a winning move.
     */
    public boolean isWinningPlay() {
        char sym = this.grid[this.lastTop][this.lastCol];
        String streak = String.format("%c%c%c%c", sym, sym, sym, sym);
        return contains(this.horizontal(), streak) ||
               contains(this.vertical(), streak) ||
               contains(this.slashDiagonal(), streak) ||
               contains(this.backslashDiagonal(), streak);
    }

    /**
     * The contents of the row containing the last played chip.
     */
    private String horizontal() {
        return new String(this.grid[this.lastTop]);
    }

    /**
     * The contents of the column containing the last played chip.
     */
    private String vertical() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; h++) {
            sb.append(this.grid[h][this.lastCol]);
        }
        return sb.toString();
    }

    /**
     * The contents of the "/" diagonal containing the last played chip
     * (coordinates have a constant sum).
     */
    private String slashDiagonal() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; h++) {
            int w = this.lastCol + this.lastTop - h;
            if (0 <= w && w < this.width) {
                sb.append(this.grid[h][w]);
            }
        }
        return sb.toString();
    }

    /**
     * The contents of the "\" diagonal containing the last played chip
     * (coordinates have a constant difference).
     */
    private String backslashDiagonal() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; h++) {
            int w = this.lastCol - this.lastTop + h;
            if (0 <= w && w < this.width) {
                sb.append(this.grid[h][w]);
            }
        }
        return sb.toString();
    }

    private static boolean contains(String haystack, String needle) {
        return haystack.indexOf(needle) >= 0;
    }

    public static void main(String[] args) {
        int height = 6, width = 8, moves = height * width;
        ConnectFour board = new ConnectFour(width, height);
        /**The emptyChar char is used for the first time the function label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function
         *is called. This is because no specific char value is needed for the first time this function is called. Due to the number in the first parameter,
         *this function sets the text of a label to tell the user(s) the acceptable range an inputed column number is supposed to be in.
         */
        char emptyChar=' ';
        board.gui.label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function(2,width,emptyChar);
        //Since the board does not already exist, the last parameter is set to false to create a new empty board.
        board.gui.createBoard(width, height, board, false);
        for (int player = 0; moves-- > 0; player = 1 - player) {
             char symbol = players[player];
             board.chooseAndDrop(symbol);
           //Since the board already does exist, the last parameter is set to true to update the existing board.
             board.gui.createBoard(width, height, board, true);
             if (board.isWinningPlay()) {
                 /**The symbol char is used for the second time the function label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function
                  *is called. Due to the number in the first parameter, this function will tell which player won. The symbol of the player who
                  *won (X or O) is stored in the symbol char. Therefore, the symbol char is used to tell which player won.
                 */
                 board.gui.label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function(1,width,symbol);
                 return;
                }
            }
      /**The empty char is used for the third time the function label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function
       *is called. This is because no specific char value is needed for the third time this function is called. Due to the number in the first parameter,
       *this function sets the text of a label to tell the user(s) that nobody has won.
      */
       board.gui.label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function(0,width,emptyChar);
    }
}
//This class GUI is used to create GUI for the game.
class GUI{

    private static JFrame jframe; //This variable is used to create a JFrame window.
    private static List<JLabel> boardLabels; //This list is used to store a group of labels related to the board's graphics.
    private static JLabel labelDisplayingWhosTurnItIs;
    private static JLabel labelTellingDetailsRegardingColumns;
    private static JLabel labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon;
    private final int defaultLabelWidth=100;
    private final int defaultLabelHeight=100;
    private final int lengthyLabelsWidth=400; //This width is used for labels that have a long width
    private final int defaultTextFieldWidth=200;
    private final int defaultTextFieldHeight=50;
//The initial x coordinate for each label in the board's graphics is multiplied by this value to add spacing among each label in the board's graphics. 
    private final int xScaling=50; 
//The initial y coordinate for each label in the board's graphics is multiplied by this value to add spacing among each label in the board's graphics. 
    private final int yScaling=50;
//After being multiplied, the x coordinate is added with xOffset to translate it to the right a little bit.
    private final int xOffset=50;
//After being multiplied, the y coordinate is added with yOffset to translate it up a little bit.
    private final int yOffset=100;
//The two variables below are used to set the size of the JFrame.
    private final int screenXSize=500;
    private final int screenYSize=900;
//When a user presses the Enter key inside the text field, this variable is set to true. 
    private static boolean inputReadyForProcessing=false;
//This is the text field where input is entered by the player.
    private static JTextField playerInput=new JTextField("");

    public GUI() {
        /** A basic GUI window is created, and variables are initialized.
         * GUI elements are positioned in coordinates by setBounds
         * and are added to the JFrame window.
         */
        jframe=new JFrame();
        boardLabels=new ArrayList<JLabel>();
        jframe.setVisible(true);
        jframe.setSize(screenXSize,screenYSize);
        jframe.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        jframe.setLayout(null);
        jframe.setResizable(false);
        labelDisplayingWhosTurnItIs=new JLabel("");
        labelDisplayingWhosTurnItIs.setBounds(150,550,defaultLabelWidth,defaultLabelHeight);
        labelTellingDetailsRegardingColumns=new JLabel("");
        labelTellingDetailsRegardingColumns.setBounds(150,600,lengthyLabelsWidth, defaultLabelHeight);
        labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon=new JLabel("");
        labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon.setBounds(150,750,lengthyLabelsWidth, defaultLabelHeight);
        playerInput.setBounds(125, 700, defaultTextFieldWidth, defaultTextFieldHeight);
        jframe.add(labelDisplayingWhosTurnItIs);
        jframe.add(labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon);
        jframe.add(labelTellingDetailsRegardingColumns);
        jframe.add(playerInput);

        /**An ActionListener is added to the text field variable. Whenever a player presses the Enter key inside this text field,
         * an action will be performed. In this case, inputReadyForProcessing is set to true, which will be useful in this program.
         */
        playerInput.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                inputReadyForProcessing=true;
            }
        });
    }

//This function can both be used to create a new empty board and to update an existing board.
public void createBoard(int width, int height, ConnectFour board, boolean updateExsistingBoard) {
        int indexOfLabel=0;
        for(int x=0; x<width; x++) {
            for(int y=0; y<height; y++) {
                /** Labels are set to the string variable slotLabelString.
                 * This string variable adds a char to an empty string, which makes it 
                 * basically equal to a string version of the char. This variable
                 * stores the value of a point in the grid array. This value is used
                 * to set each label equal to its corresponding point in the grid array.
                 * The grid array is used in the backbone for this program for basic 
                 * game logic.
                 */
                String slotLabelString=board.grid[y][x]+"";  
                if(updateExsistingBoard) {
                    //If the board already exists, just update the label's text.
                    boardLabels.get(indexOfLabel).setText(slotLabelString);
                }
                else {
                    //If the board does not already exist, create a new label.
                    boardLabels.add(new JLabel(slotLabelString));
                    boardLabels.get(indexOfLabel).setBounds(x*xScaling+xOffset,y*yScaling+yOffset,defaultLabelWidth,defaultLabelHeight);
                }
                //Each label is added to the JFrame window.
                jframe.add(boardLabels.get(indexOfLabel));
                indexOfLabel++;
            }
        }
    }

//This function can change the text of a specific label.
public void label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function(int did_Someone_Win_One_For_Yes_Two_For_No_Other_For_Draw, int width, char symbol) {
    if (did_Someone_Win_One_For_Yes_Two_For_No_Other_For_Draw==1) { 
        labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon.setText("Player " + symbol + " wins!");
    }
    else if (did_Someone_Win_One_For_Yes_Two_For_No_Other_For_Draw==2) {    
        labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon.setText("Use 0-" + (width - 1) + " to choose a column.");
    }
    else {
        labelThatTellsPlayerWhatColumnsTheyCanPickOrTellsThemWhoWon.setText("Game over, no winner.");
    }

}

/**This function is used to return the integer version of the string entered in the text field.
 *This function is called in the chooseAndDropGUI from the ConnectFour class.
 */
public int readPlayerInputForProcessing() 
{
    while(true) 
    {   
        int playerInputValue;

        /**If a player entered an input that is not a number, trying to convert it into an integer would result in errors.
         * Therefore, the try and catch method is used.
         */
        try 
        {
            playerInputValue=Integer.valueOf(playerInput.getText());
        }

        catch(Exception err)
        {
            //If an error is caused, the program goes back to the beginning of the while loop.
            continue;
        }

        /**This if statement is intentionally not the first if statement in the function.
         *This allows the function to have some time to process if whether inputReadyForProcessing is equal to true or not.
         *This variable is equal to true only if an Enter key was pressed in the text field. Therefore, the playerInputValue
         *is only returned when a user presses the Enter key. Otherwise, the program goes back to the beginning of the while loop.
         */
        if(!inputReadyForProcessing) 
        {
            continue;
        }
        /** This variable is set back to its initial state, false. To make this variable true again, the Enter key will have to be
         * pressed inside the text field once again. This makes the program only return numerical input inside the text field only when 
         * the Enter key is pressed inside the text field.
         */
        inputReadyForProcessing=false;

        return playerInputValue;
    }
}
/**
 * This function is created to create the GUI aspects for the function chooseAndDropGUI in the ConnectFour class. 
 */
public void chooseAndDropGUI(char symbol, String columnInfo) {
    labelDisplayingWhosTurnItIs.setText("\nPlayer " + symbol + " turn: ");
    labelTellingDetailsRegardingColumns.setText(columnInfo);
}

}
\$\endgroup\$
  • \$\begingroup\$ The official recommendation is to prefer JavaFX to Swing. Are you sure this is where you want to put your energy? \$\endgroup\$ – Eric Stein Aug 19 at 19:16
2
\$\begingroup\$

Separation of concerns

As you stated, the major edit in this iteration was adding a graphical presentation.The principle of Separation of concerns dictates (among other things) the Separation of content and presentation. So the proper design for this game would be to create a "game engine" that is responsible for the logic of the game and publishes an API. another API, the presentation API, would allow the same game engine to be plugged with different UI implementations. the main() method is usually the place where you construct the implementations (based on run time args, config files or the like) and make the necessary connections between the layers.

However, this is not the case here. ConnectFour class initializes an instance of GUI and cannot work with any other UI. Furthermore, there are presentation aspects coded into the class. For example, the players' characters ('X' and 'O'), error messages (columnInfo)

Naming conventions

public void label_That_Tells_Player_What_Columns_They_Can_Pick_Or_Tells_Them_Who_Won_Function(int did_Someone_Win_One_For_Yes_Two_For_No_Other_For_Draw, int width, char symbol)

not only this name violates Java's camelCase naming convention. it is simply a bad name for a function/method in any programming language. first of all, the method name describes a label but doesn't say what is being done to the label (is it reading or writing to/from the label?). second, a rule of thumb states that method (and variable and class) name should not exceed 20+ characters. longer names do not add clarity. that's for documentation in comments.

regarding did_Someone_Win_One_For_Yes_Two_For_No_Other_For_Draw - you don't document the valid values of a variable in it's name. there are two better places to do that: 1) in comments, and 2) create an enum that restricts valid values and also gives meaningful names to each valid value.

\$\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.