Thread: Simple chess program (no AI) - Need help with structuring my code

Hi everyone. I hope this is the right forum to post this.

I'm not really good at java and this is my first project.

I'm trying to make a simple chess program. It's not finished yet. What you basically can do is to move a pawn from one square to another.

I need help on how to structure the code. I find myself mixing a lot of gui stuff with logic but I really don't know how to do it properly. I guess my code is also kind of badly written. I could really use some suggestions on what to do different.

import javax.swing.*;
import java.awt.*;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.event.MouseMotionListener;
 
public class Chess extends JFrame {
	public Piece[] piece = new Piece[32];
	private GridBagConstraints c = new GridBagConstraints();
	private HandlerClass handler = new HandlerClass();
	private int[][] position = new int[8][8];
	private JPanel boardPanel = new JPanel();
	private JPanel[][] squares = new JPanel[8][8];
	private Point hotSpot = new Point(15, 20);
	private Toolkit toolkit = Toolkit.getDefaultToolkit();
	private int[] initCoord = new int[2];
 
	public static void main(String[] args) {
 
		new Chess();
 
	}
 
	Chess() {
		setVisible(true);
		setDefaultCloseOperation(EXIT_ON_CLOSE);
		JPanel mainPanel = new JPanel();
 
		boardPanel.setLayout(new GridBagLayout());
		boardPanel.addMouseListener(handler);
		mainPanel.setBackground(Color.BLACK);
 
		mainPanel.add(boardPanel);
		add(mainPanel);
 
		createBoard();
		createPieces();
		updatePosition();
 
		// setSize(600, 600);
		pack();
		setResizable(true);
	}
 
	// Grid of JPanel with 8x8 rows and columns which are added to the
	// boardPanel
	private void createBoard() {
		for (int i = 0; i < 8; i++) {
			for (int j = 0; j < 8; j++) {
				squares[i][j] = new JPanel();
 
				if (((i & 1) == 0 && (j & 1) == 1)
						|| ((i & 1) == 1 && (j & 1) == 0))
					squares[i][j].setBackground(Color.red);
				else
					squares[i][j].setBackground(new Color(211, 240, 254));
 
				squares[i][j].setPreferredSize(new Dimension(75, 75));
 
				c.gridx = j;
				c.gridy = i;
				c.ipady = 0;
				c.ipadx = 0;
				boardPanel.add(squares[i][j], c); // Adds the current squares to
													// boardPanel
			}
		}
	}
 
	// Calls piece class to create pieces. At the moment only white pawns.
	// The piece constructor needs to know whether piece should be black or white. 
	// A new parameter will be added to fix that (will be done later).
	private void createPieces() {
		for (int i = 0; i < 8; i++) {
			piece[i] = (Piece) new Pawn(i, 1);
		}
	}
 
	// Updates the positions of the pieces. This method will be initially called and then every time a piece is moved.
	private void updatePosition() {
		deletePosition(); // First clear all the all positions, then rebuild the array with updated ones.
		for (int i = 0; i < 8; i++) {
			squares[piece[i].getY()][piece[i].getX()].add(piece[i]
					.getPieceLabel());
			storePosition(piece[i].getX(), piece[i].getY(), i);
		}
 
	}
 
	// To keep track of where the pieces are, the updatePosition method calls this one with the
	// current piece's position which is then stored in an 8x8 array. The value corresponding to each index
	// is the piece number (int k, defined in the createPieces()-method (in that method its called i))
	private void storePosition(int i, int j, int k) {
		position[i][j] = k;
	}
 
	// Array values are by default 0. This is why I need to change them to -1. If I didn't, it would seem like piece[0] would be in every
	// unoccupied square.
	private void deletePosition() {
		for (int i = 0; i < 8; i++) {
			for (int j = 0; j < 8; j++) {
				position[i][j] = -1;
			}
		}
	}
 
	// Kind of unnecessary because the position[][]-array is "transparent" through the class.
	private int getPosition(int x, int y) {
		return position[x][y];
	}
 
	// MOUSE EVENTS :
 
	// Every square is 75x75 pixels. This method returns the square coordinates based on where i click on the board.
	private int coord(int x) {
		if (x >= 0 && x < 75) {
			x = 0;
		} else if (x >= 75 && x < 150) {
			x = 1;
		} else if (x >= 150 && x <= 225) {
			x = 2;
		} else if (x >= 225 && x <= 300) {
			x = 3;
		} else if (x >= 300 && x <= 375) {
			x = 4;
		} else if (x >= 375 && x <= 450) {
			x = 5;
		} else if (x >= 450 && x <= 525) {
			x = 6;
		} else if (x >= 525 && x <= 600) {
			x = 7;
		}
 
		return x;
	}
 
	// Store initial positions of moving piece
	private void storeInit(int x, int y) {
		initCoord = new int[2];
		initCoord[0] = x;
		initCoord[1] = y;
	}
 
	private class HandlerClass implements MouseListener, MouseMotionListener {
		boolean test = false;
 
		@Override
		public void mouseDragged(MouseEvent e) {
			// TODO Auto-generated method stub
 
		}
 
		@Override
		public void mouseMoved(MouseEvent e) {
			// TODO Auto-generated method stub
 
		}
 
		@Override
		public void mouseClicked(MouseEvent e) {
		}
 
		@Override
		public void mouseEntered(MouseEvent e) {
			// TODO Auto-generated method stub
 
		}
 
		@Override
		public void mouseExited(MouseEvent e) {
			// TODO Auto-generated method stub
 
		}
 
		@Override
		public void mousePressed(MouseEvent e) {
			int x = coord(e.getX()); //Calls the coord() method to convert coordinates on the board to square coordinates
			int y = coord(e.getY());
 
			if (position[x][y] != -1) {
				setCursor(toolkit.createCustomCursor(piece[position[x][y]]
						.getIcon().getImage(), hotSpot, "")); // "Drag and drop": change cursor to piece image
				piece[position[x][y]].getPieceLabel().setVisible(false); // then hide the piece until dropped on new square (see mouseReleased())
				storeInit(x, y);
				test = true;
			}
 
		}
 
		@Override
		public void mouseReleased(MouseEvent e) {
			int x = coord(e.getX());
			int y = coord(e.getY());
			setCursor(Cursor.getPredefinedCursor(Cursor.DEFAULT_CURSOR)); // Set default cursor as soon as mouse is released
			if (test) { // If there was a piece to move, set that piece to its new coordinates
				piece[getPosition(initCoord[0], initCoord[1])].setX(x); // Here is where the storeInit() method comes in. 
				piece[getPosition(initCoord[0], initCoord[1])].setY(y);
				piece[getPosition(initCoord[0], initCoord[1])].getPieceLabel()
						.setVisible(true);
				updatePosition();
				repaint();
				test = false;
			}
 
		}
 
	}
 
}

Piece class:

import javax.swing.ImageIcon;
import javax.swing.JLabel;
 
 
public abstract class Piece {
	public abstract int getX();
	public abstract int getY();
	public abstract String getType();
	public abstract ImageIcon getIcon();
	public abstract JLabel getPieceLabel();
 
	public abstract void setX(int x);
	public abstract void setY(int y);
 
}

Pawn class:

import javax.swing.ImageIcon;
import javax.swing.JLabel;
 
 
public class Pawn extends Piece {
 
	private int x;
	private int y;
	private ImageIcon icon;
	private JLabel pieceHolder;
 
	public Pawn(int x, int y) {
		this.x = x;
		this.y = y;
		this.icon = new ImageIcon("PATH_TO_IMAGE"); // REPLACE WITH IMAGE PATH
		this.pieceHolder = new JLabel();
		this.pieceHolder.setIcon(icon);
	}
 
	@Override
	public int getX() {
		return this.x;
	}
 
	@Override
	public int getY() {
		return this.y;
	}
 
	@Override
	public String getType() {
		return "Pawn";
	}
 
	@Override
	public ImageIcon getIcon() {
		return this.icon;
	}
 
	public JLabel getPieceLabel() {
 
		return this.pieceHolder;
	}
 
	@Override
	public void setX(int x) {
		this.x = x;
	}
 
	@Override
	public void setY(int y) {
		this.y = y;
	}
 
}

To run program, you need an image of a pawn (or whatever you want to use instead). I used this one http://upload.wikimedia.org/wikipedi..._plt45.svg.png - download and then in the Pawn class: this.icon = new ImageIcon("PATH_TO_IMAGE"); (for some reason i had to put the whole path, even if the image was in the same folder as the class-file).

Oh, and one more question: Should I make a new class for the rules (to see if a move is valid, castleing, en passant etc) or is it better to do this in each piece's class?

One suggestion I have: instead of defining x, y, icon, and placeHolder in the Pawn class, do it in the Piece class. Every Piece you make is going to have these fields, and it's redundant to redefine them in every class, especially since you're using a superclass. Furthermore, putting these fields in the superclass will allow you to get rid of almost all of its abstract methods and define those methods in the superclass--saving you a ton of work.

Should I make a new class for the rules (to see if a move is valid, castleing, en passant etc) or is it better to do this in each piece's class?

Truly, that is up to you. You could make an abstract method in the Piece class that takes information about all the pieces on a chess board and then spits out the possible valid moves. Or, you could create a class with methods that accept pieces and chess board information and have all move logic flow through it. Whatever makes the most sense to you is what you should do.

You've got it partly. Now, think about
private ImageIcon icon;

You said

HOWEVER, I didn't include the "ImageIcon icon" as it depends on the piece-type.

which makes sense, but think about this: though each piece has a different ImageIcon, EVERY piece has one. If every single piece has one, no matter what, why not define the ImageIcon in the superclass? No one says you have to explicitly define it in the superclass's constructor; you could even leave it as an argument in the superclass constructor and then have the subclasses "fill it in." To show what I mean, look at this:

/**
* Every Ball has a size, but the size is different for every subclass
* of Ball. So, what I'm going to do is define int size in the Ball class
* and put it in Ball's constructor so that subclasses can access it.
*/
public class Ball {
  private String manufacturer;
  private int radius; //let's say in inches
 
  Ball(String maker, int radius) {
    manufacturer = maker;
    this.radius = radius;
  }
}

/**
* Now look at the subclass's constructor: it only takes one argument.
* Of course, I'm assuming for simplicity that all soccer balls have the 
* same radius, but look at how the subclass constructor fills in the 
* int radius parameter of the superclass constructor.
*/
public class SoccerBall extends Ball {
  public SoccerBall(String maker) {
    //The subclass uses 7 as the assumed radius for soccer balls
    super(maker, 7);
  }
}

By doing this, I tell Java that all implementations of Ball will have a radius, allow the superclass to define methods using radius, and save myself a ton of work. I suggest doing the same with your ImageIcon. Hopefully that made sense? Sometimes I'm a poor explainer.

Now, the other thing I think you should change:

public abstract int getX();
public abstract int getY();
public abstract String getType();
public abstract ImageIcon getIcon();
public abstract JLabel getPieceLabel();

public abstract void setX(int x);
public abstract void setY(int y);

You make these methods abstract; why? You already define x, y, pieceLabel, and (you should ) icon in the superclass, so why not just define these methods right in the superclass? The variables will store different values depending on the subclass, but no matter what, pieceLabel will always be a JLabel and x and y will always be ints. Defining in the superclass the methods to get and set these variables will in no way hamper the differences among the subclasses--in fact, as is a common theme in my post , it will save you time and effort.

Originally Posted by snowguy13

Now, the other thing I think you should change:

public abstract int getX();
public abstract int getY();
public abstract String getType();
public abstract ImageIcon getIcon();
public abstract JLabel getPieceLabel();

public abstract void setX(int x);
public abstract void setY(int y);

You make these methods abstract; why? You already define x, y, pieceLabel, and (you should ) icon in the superclass, so why not just define these methods right in the superclass? The variables will store different values depending on the subclass, but no matter what, pieceLabel will always be a JLabel and x and y will always be ints. Defining in the superclass the methods to get and set these variables will in no way hamper the differences among the subclasses--in fact, as is a common theme in my post , it will save you time and effort.

Hi. Thank you very much for your reply!
I think I got i now

This is the changes I made:

Piece-class:

import javax.swing.ImageIcon;
import javax.swing.JLabel;
 
public abstract class Piece {
	protected int x;
	protected int y;
	protected String color;
	protected JLabel pieceHolder;
	protected ImageIcon icon;
	protected String type;
 
	public Piece(int x, int y, String color) {
		this.x = x;
		this.y = y;
		this.color = color;
		this.pieceHolder = new JLabel();
	}
 
	public int getX() {
		return this.x;
	}
 
	public int getY() {
		return this.y;
	}
 
	public String getType() {
		return type;
	}
 
	public ImageIcon getIcon() {
		return this.icon;
	}
 
	public JLabel getPieceLabel() {
		return this.pieceHolder;
	}
 
	public String getColor() {
		return color;
	}
 
	public void setX(int x) {
		this.x = x;
	}
 
	public void setY(int y) {
		this.y = y;
	}
 
}

Pawn-class:

import javax.swing.ImageIcon;
 
public class Pawn extends Piece {
	public Pawn(int x, int y, String color) {
		super(x, y, color);
		icon = new ImageIcon("PATH/TO/IMAGE/FOLDER" + color
				+ "Pawn.png");
		type = "Pawn";
		this.pieceHolder.setIcon(icon);
	}
}

I fixed the ImageIcon "issue" aswell. But I still add the image in Pawn-constructor. Is it preferable to add it in the Piece-class?

Edit: I guess it's no point in using an abstract class now? I changed public abstract class Piece to public class Piece and it made no difference

Yeah, that looks a lot better!

But I still add the image in Pawn-constructor. Is it preferable to add it in the Piece-class?

What you're doing now looks fine, so long as it is easy for you to understand. If another organization seems more logical to you, then I'd do that.

I guess it's no point in using an abstract class now?

Yeah, all the abstract methods are gone.

Good luck with your project!