6
\$\begingroup\$

I am making a Connect Four type of game in Java. I want to make sure that all the code seems nice before I decide to add more.

This is what the applet game looks like:

Player input:

input

Victory screen:

win


I have three classes:

Make4:

import java.util.Arrays;
import java.awt.*;
import java.applet.Applet;
import javax.swing.JOptionPane;


/**
 * @since 0.1.0
 */
public class Make4 extends Applet {
Board gb = new Board();
boolean hasFirstRun;

public void start(Graphics g) {
    hasFirstRun = false;
}

public void paint(Graphics g) {
    paintBoard(g);
    if(!hasFirstRun) {
        hasFirstRun = true;
        startGame(2,g);
    }
}

public void paintBoard(Graphics g) {
    Expo.setColor(g,Expo.black);
    for(int y = 0; y<gb.HEIGHT*100; y+=100) {
        Expo.drawLine(g,0,y,gb.WIDTH*100,y);
    }
    for(int x = 0; x<gb.WIDTH*100; x+=100) {
        Expo.drawLine(g,x,0,x,gb.HEIGHT*100);
    }

    for(int y = 0; y<gb.HEIGHT; y++) {
        for(int x = gb.WIDTH-1; x>=0; x--) {
            if(gb.board[x][y] != 0) {
                switch(gb.board[x][y]) {
                    case 1:
                        Expo.setColor(g, Expo.red);
                    break;
                    case 2:
                        Expo.setColor(g,Expo.black);
                    break;
                    case 3:
                        Expo.setColor(g,Expo.green);
                    break;
                    case 4:
                        Expo.setColor(g,Expo.purple);
                    break;
                }
                Expo.fillCircle(g,x*100+50,y*100+50,45);
            }
        }
    }
}

public void startGame(int np, Graphics g) {
    int player = 0;
    int numPlayer = np;
    int wonType = 0;

    while(wonType == 0) {

        if(player<numPlayer) {
            player++;
        } else {
            player = 1;
        }

        paintBoard(g);
        getPlayerInput(player);
        wonType = gb.checkWin(player);
    }

    paintBoard(g);

    switch(wonType) {
        case 1:
            JOptionPane.showMessageDialog(null, "Player "+player+" won!");
        break;

        case 2:
            JOptionPane.showMessageDialog(null, "There was a draw!");
        break;

        default:
            JOptionPane.showMessageDialog(null, "There was an error with the win type...\nPlease contact the developer.");
        break;
    }
}

public void getPlayerInput(int p) {  // diffrent class
    int player = p;
    try {
        String tempString = JOptionPane.showInputDialog("Player " + player + "\'s turn: ");
        int playerColumn = Integer.parseInt(tempString);

        if(playerColumn>gb.WIDTH) {
            throw new Exception();
        } 

        gb.dropPiece(playerColumn, player);
    } catch(ArrayIndexOutOfBoundsException ex) {
        JOptionPane.showMessageDialog(null, "That collum appears to be full. please try a diffrent one");
        getPlayerInput(player);
    } catch(Exception e) {
        JOptionPane.showMessageDialog(null, "Please input a valid collum between 1 and "+gb.WIDTH);
        getPlayerInput(player);
    }
}
}

Board

import java.util.Arrays;

/**
 * @since 0.1.0
 */
class Board {
final int WIDTH;
final int HEIGHT;

int[][] board;

public Board() {
    board = new int[7][6];
    WIDTH = board.length;
    HEIGHT = board[0].length;
}

public Board(int w, int h) {
    board = new int[w][h];
    WIDTH = board.length;
    HEIGHT = board[0].length;
}

public void dropPiece(int x, int player) {
    int xwidth = x-1;
    int xheight = HEIGHT-1;
    while (board[xwidth][xheight] != 0) {
        xheight--;
    }
    board[xwidth][xheight] = player;
    //System.out.println("Player placed piece at: " + xwidth + "," + xheight);
}

public int checkWin(int player) {
    if(isDraw()) {
        return 2;
    }

    for (int x = 0; x < WIDTH; x++) {
        for (int y = 0; y < HEIGHT; y++) {
            if (board[x][y] != player) {
                continue; 
            }

            if (x + 3 < WIDTH && player == board[x+1][y] && board[x][y] == board[x+2][y] && board[x][y] == board[x+3][y]) {
                return 1;
            }

            if (y + 3 < HEIGHT) {
                if (board[x][y] == board[x][y+1] && board[x][y] == board[x][y+2] && board[x][y] == board[x][y+3]) {
                    return 1;
                }
                if (x + 3 < WIDTH && board[x][y] == board[x+1][y+1] && board[x][y] == board[y+2][y+2] && board[x][y] == board[y+3][y+3]) {
                    return 1;
                }
                if (x - 3 >= 0 && board[x][y] == board[x-1][y+1] && board[x][y] == board[x-2][y+2] && board[x][y] == board[x-3][y+3]) {
                    return 1;
                }
            }
        }
    }
    return 0;
}

public boolean isDraw() {
    for(int x = 0; x < WIDTH; x++) {
        for(int y = 0; y < HEIGHT; y++) {
            if(board[x][y] == 0) {
                return false;
            }
        }
    }
    return true;
}
}

and this class from my computer science class (I took out all the things I'm not using):

import java.awt.*;
import java.util.*;
import java.applet.Applet;

public class Expo
{
static final Color red            = Color.red;
static final Color green          = Color.green;
static final Color purple         = new Color(128,0,128);
static final Color black          = Color.black;
static Color currentColor         = black;
public static void setColor(Graphics g, Color newColor)
{
    g.setColor(newColor);
    currentColor = newColor;
}
public static void drawLine(Graphics g, int x1, int y1, int x2, int y2)
{
    g.drawLine(x1,y1,x2,y2);
}
public static void fillCircle(Graphics g, int centerX, int centerY, int radius)
{
    int diameter = 2 * radius;
    g.fillOval(centerX-radius, centerY-radius, diameter, diameter);
}
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

You seem to have a number of names that are very nondescript. What is Expo? And hasFirstRun in what marathon? You at least need to include some comments.

I notice that you have some magic numbers floating around. 2 doesn't exactly scream "draw", so you should use an enum.


You have also misspelled column as: collum, in two different output statements. It's not a code problem, but I'm mentioning it anyway.


Java, for whatever reason, has not yet deprecated the lowercase colors. They do not conform to the convention of uppercase constants. I would change:

Color.red;

to

Color.RED;

You should also change the names of the constant variables to uppercase.

See here: https://stackoverflow.com/questions/7281180


I'm also noticing some minor formatting inconsistencies that hurt readability a little. Expo lacks a layer of indentation.

And this code could use some spacing:

for(int y = 0; y<gb.HEIGHT; y++) {
    for(int x = gb.WIDTH-1; x>=0; x--) {

Spacing added:

for(int y = 0; y < gb.HEIGHT; y++) {
    for(int x = gb.WIDTH - 1; x >= 0; x--) {

There are a few more spots that need similar treatment, but this could be automatically done by an IDE.


The other thing is that you have some unnecessary code duplication here:

public Board() {
    board = new int[7][6];
    WIDTH = board.length;
    HEIGHT = board[0].length;
}

public Board(int w, int h) {
    board = new int[w][h];
    WIDTH = board.length;
    HEIGHT = board[0].length;
}

You could just have:

public Board() {
    this(7,6);
}

public Board(int w, int h) {
    board = new int[w][h];
    WIDTH = board.length;
    HEIGHT = board[0].length;
}
\$\endgroup\$
2
  • \$\begingroup\$ Expo is a class that is provided with my computer science book. Basically, it takes a lot of Java features and simplifies them (I remove all the methods but the ones I'm using) I think the name came from the fact that most of the class works with graphics, so it is referring to expo markers. I will definitely add comments and try to be more descriptive with my variables \$\endgroup\$ Commented May 15, 2016 at 1:30
  • 1
    \$\begingroup\$ @Smarticles101 I find that textbooks have awful code. I have seen some stuff in mine that has blatant logic errors. \$\endgroup\$
    – Laurel
    Commented May 15, 2016 at 1:34

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