HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Go (board game) in Java

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javagameboard

Problem

Go is a board game (it looks like 5-in-a-row and plays like chess) and I tried to program it in Java.

Rules:

Two players take turn placing stones on a board. The goal is to capture teritory. Stones can be captured (removed from game) if they have no unoccupied adjacent tiles horizontaly/verticaly. Adjacent stones (horizontaly/verticaly) of one color are combined into chains which share unoccupied tiles.

If you're interested here's more.

My code:

I implemented core rules and basic input/output. My goal is to make the game scalable so I can add functionality without breaking everything.

My concerns:

  • (main concern) I don't think it's expandable (enough). I had to redo (nearly) everything from scratch because it was complete mess. It works but I think that if I add more game mechanism everything will break. What can I do to avoid this?



  • Is Main an acceptable name for a class? Or should it be Game/App or something completely different?



  • What about my comments?



Main

package go;

import java.awt.BorderLayout;
import java.awt.Color;

import javax.swing.BorderFactory;
import javax.swing.JFrame;
import javax.swing.JPanel;

/**
 * Builds UI and starts the game.
 *
 */
public class Main {

public static final String TITLE = "";
public static final int BORDER_SIZE = 25;

public static void main(String[] args) {
    new Main().init();
}

private void init() {
    JFrame f = new JFrame();
    f.setTitle(TITLE);

    JPanel container = new JPanel();
    container.setBackground(Color.GRAY);
    container.setLayout(new BorderLayout());
    f.add(container);
    container.setBorder(BorderFactory.createEmptyBorder(BORDER_SIZE, BORDER_SIZE, BORDER_SIZE, BORDER_SIZE));

    GameBoard board = new GameBoard();
    container.add(board);

    f.pack();
    f.setResizable(false);
    f.setLocationByPlatform(true);
    f.setVisible(true);
}}


GameBoard

```
package go;

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import j

Solution

Not a full review, just a couple of initial thoughts:


Is "Main" acceptable name for a class? Or should it be "Game"/"App" or something completely different?

I would say "Main" is acceptable if it doesn't to anything except start the app; it doesn't contain any app logic itself. I think for your game this is pretty much true, but I would probably extract the init stuff into a MainGUI or ContainerGUI class (because it will most likely grow soon, eg by adding a menu, customizable view, etc).


What about my comments?

Your JavaDoc comments seem mostly good to me, but your inline comments are often not necessary.

  • Check wherever it's valid is a bit unclear. What is it? Where is it valid? I think bounds check would be clearer, but then it becomes obvious that the comment isn't actually needed.



  • @param black for State state is wrong (I'm assuming you had it as a boolean first?)



  • Switch current player It's often not a good idea to write comments that just repeat the code. If you think your code is unclear/you want more structure, it's often better to create a method for it.



  • what's a liberty? A comment for eg getLiberties about this would be nice.



  • join(Chain chain) could use a short comment



  • Row and col are need to remove (set to null) this Stone from Grid this could be clearer.




I don't think it's expandable (enough).

I think it looks pretty good. There are a couple of things I would do differently (eg separate view and controller, remove the small parts of game logic - switching players - that are in the view/controller, and some of the stuff in misc), but all in all it has a clear structure.

Misc

  • Chain(State state): state is actually never used.



  • don't shorten variable names, it makes code harder to read. N_OF_TILES -> NUMBER_OF_TILES/TILE_COUNT



  • why are all fields of Stone public instead of having getters/setter?



  • State is a very generic term. Player or PlayerColor/StoneColor may make more sense.



  • I would extract the MouseAdapter to it's own class, in case it gets more complicated later on.



  • you could avoid the state != null check by creating a None value for State, I think this would result in nicer code.

Context

StackExchange Code Review Q#93901, answer score: 11

Revisions (0)

No revisions yet.