Go (board game) in Java -- version 2

First version: Go (board game) in Java

What's new:

• I now use (as advised) HashMap where GoPoint is my "coordinate" class and StoneColor is an enum.

• Game allows only legal moves.

• Player can pass (skip turn).

• Player can choose the size of game board.

• Code is more readable (I hope).

• Better separation of view and model.

• Everything should work and you can actually play.

What's missing:

• Score counting. I'm not even sure wherever I'll implement it. Any advice on how to do it (that's not too complicated if possible) is welcome.

• A.I. This is not going to happen in near future. I definitely want to try building A.I. but I'll try it with something simpler.

Anything that could improve my code is welcome. Is it readable? Should I do something diferently?...

GoMain

```
package go;

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.event.ActionEvent;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;

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

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

public static final String TITLE = "Simple Go";
public static final int OUTSIDE_BORDER_SIZE = 25;

private StartDialog startDialog;

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

private void init() {
startDialog = new StartDialog(this);
startDialog.pack();
startDialog.setLocationByPlatform(true);
startDialog.setVisible(true);
}

public void startGame(int size) {
JFrame f = new JFrame();
f.setTitle(TITLE);

f.add(createMainContainer(size));

f.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
f.addWindowListener(new WindowAdapter() {
@Override
public void windowClosing(WindowEvent e) {
super.windowClosing(e);

Just some comments on your GoPoint class...

• Good: it's a final class with final fields.

-
Not so good:

@Override
public boolean equals(Object obj) {
    if (this == obj)
        return true;
    if (obj == null)
        return false;
    if (getClass() != obj.getClass())
        return false;
    GoPoint other = (GoPoint) obj;
    if (col != other.col)
        return false;
    if (row != other.row)
        return false;
    return true;
}

This can be simplied as as just:

@Override
public boolean equals(Object obj) {
    if (obj instanceof GoPoint) {
        GoPoint o = (GoPoint) obj;
        return col == o.col && row == o.row;
    }
    return false;
}

Also, since you are using final fields, you can consider pre-calculating/setting your hashCode() and toString() values first.

edit

Pre-calculating/setting what are invariant results first can lead to better performance, since you avoid repetition. What I'm suggesting is:

public final class GoPoint {

    private final int row;
    private final int col;
    private final int hashCode;
    private final String toString;

    public GoPoint(int row, int col) {
        this.row = row;
        this.col = col;
        this.hashCode = calculateHashCode();
        this.toString = "GoPoint [row=" + row + ", col=" + col + "]";
    }

    private int calculateHashCode() {
        final int prime = 31;
        return prime * (prime * col) + row;
    }

    @Override
    public int hashCode() {
        return hashCode;
    }

    @Override
    public String toString() {
        return toString;
    }
}

Another observation:


I now use (as advised) HashMap...

That doesn't mean you have to declare them as HashMap. :) Declaration by interface (Map map = new HashMap<>();) is recommended as it lets you abstract away implementation details from users of the field - they only need to know they are interfacing with a Map instance, and you are free to replace with different implementations down the road as you deem fit.

StackExchange Code Review Q#94556, answer score: 5