1
\$\begingroup\$

As a high school assignment, I was asked to make a Connect 4 game in Java. I have created a 9×9 2-D array that two players can play. I'd appreciate any feedback or suggestions.

package Connect4;

import java.util.Scanner;
public class Connect4 {


    public static void main(String[] args) {
        Scanner input=new Scanner(System.in);
        int count=0,count2=0,count3,pn=2;
        String choice;
        int[][]board=new int[9][9];
        boolean isWin=false;
        System.out.println("--Connect 4--\n\nHow to play:\n\nPlayers 1 and 2 alternate as they select a column to drop their chip in.\n\nConnect 4 chips in a row either vertically, horizontally, or diagonally to win.\n");
        while(!isWin){

            if(pn==2)pn=1;//Changes players' turn
            else pn=2;//Changes players' turn

            display(board,count2);

            System.out.print("Player "+pn+" enter your column choice: ");
            choice=input.nextLine();
            choice=choice.toUpperCase();

                  for(count=0;!"ABCDEFGHI".contains(choice);count++){//Making sure that their column entry is into a valid column.
                      System.out.print("This is not a valid column entry. Please enter a valid column: ");
                      choice=input.nextLine();
                      choice=choice.toUpperCase();
                  }


                  count=8;

                  int ascii=(int)choice.charAt(0)-65;//Converts their letter input to a number in order to coordinate with the columns.   

                  for(count2=0;count2<board.length;count2++){//Checks if column is full.

                      if(board[0][ascii]!=0){
                          System.out.print("This column is full. Please enter another colum: ");
                          choice=input.nextLine();
                          choice=choice.toUpperCase();

                          if(count!=8)count++;//Makes sure that array doesn't go out of bounds.
                      }

                      ascii=(int)choice.charAt(0)-65;

                      if(board[count][ascii]>0){//Makes sure that values in column stack.
                          count--;
                      }

                      else{ 
                          board[count][ascii]=pn;//Adds their input into the array.

                          for(int r=0 ; r<board.length;r++){//Checks the array for values.
                              for(int c=0 ; c<board[0].length;c++){
                                  if(board[r][c]==pn){
                                      isWin=checkWin(board,pn,r,c);
                                      if(isWin)break;
                                  }
                              } 
                              if(isWin)break;
                          }
                          break;
                      }
                  }              
        }

        if(isWin){
            for(count=0;count<board.length;count++){
                for(count2=0;count2<board.length;count2++){
                    System.out.print(board[count][count2]+" ");
                }
                System.out.println();   
            }
            System.out.println("A B C D E F G H I");
            System.out.println("Player "+pn+" wins!");
        }



    }
    public static void display(int board[][],int count2){
                for(int count=0;count<board.length;count++){
             for(count2=0;count2<board.length;count2++){
            System.out.print(board[count][count2]+" ");
            }
            System.out.println(); 

        }
                System.out.println("A B C D E F G H I");
    }
    public static boolean checkWin(int[][] board,int pn,int x, int y){
    boolean win = false;

    // down, right, upright, downright
    int[] row={1,0,-1,1};
    int[] col={0,1, 1,1};
    int count, intR,intC;
     for(int d=0;d<4;d++){ //four possible directions
         count=1;           //reset values to originals for each direction
         intR=x;
         intC=y;
         for(int c=1;c<=3;c++){ //check boundaries with next increment
            if((intR+row[d]>=0 && intR+row[d]<board.length) && (intC+col[d]>=0 && intC+col[d]<board[0].length)){
                intR+=row[d];   //increment is within the boundaries so move to that spot
                intC+=col[d];
                if(board[intR][intC]==pn)count++;  //count a correct spot
                else break; //incorrect digit found
            } else break; //break stops the loop
         }                //didn't fall within boudaries
         if(count==4){win=true;break;}  //a count of 4 indicates a win
     }
    return win;
   }
}
\$\endgroup\$
1
\$\begingroup\$

Thank you for sharing your code.

Here are my thoughts about it.

OOP

Your code is a procedural approach to the problem.

There is nothing wrong with procedural approaches in general, but Java is an object oriented (OO) programming language and if you want to become a good java programmer then you should start solving problems in an OO way.

best practices

Separation of Concerns (SoC)/ Single Responsibility Pattern (SRP)

Your main method is huge and does a lot of things. But a Method (or class) should do only one thing (and do that well).

On the other hand, if you look at your main method, you can identify at least 4 "blocks". You should extract those logical blocks out to methods of their own.

This way, your main method will also comply to the same level of abstraction (SLA) principle as described in the next section.

same level of abstraction (SLA)

SLA means that methods (and later on, classes) should either do primitive operations or call other methods (on other objects), but not both at the same time.

in your main method you do primitive operations like

        if (pn == 2)
            pn = 1;// Changes players' turn
        else
            pn = 2;// Changes players' turn

and call other methods like

        display(board, count2);

You should extract all logical blocks of a method to methods their own (as suggested in the previous section). This way your main could read out like this (roughly):

public static void main(String[] args) {
    Scanner input = new Scanner(System.in);
    int count = 0, count2 = 0, count3, pn = 2;
    String choice;
    int[][] board = new int[9][9];
    boolean isWin = false;
    System.out.println(
            "--Connect 4--\n\nHow to play:\n\nPlayers 1 and 2 alternate as they select a column to drop their chip in.\n\nConnect 4 chips in a row either vertically, horizontally, or diagonally to win.\n");
    while (!isWin) {
        switchPlayer();           
        display(board, count2);
        requestUserInput();
        addCoinToColumn();
     }
     checkWin();
}

avoid break

You leave the while loop with break. This looks like a good and short solution but in the long run it will stand in your way if you want to improve your code.

So it does here.

You can't simply extract the the individual loops to methods of their own since this may break your logic.

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