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

Slide-Game in Swing

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

Problem

This was done as an assignment in school. As I am still learning some of the basic parts of Java be merciless when it comes to my code. As it is an assignment, some things are unchangeable. The specification requires:

  • Swing. So no third-party library is allowed.



  • Cell class with getCol() and getRow() methods.



  • For the final product to be a Slide-Game.



Some extra features that I tried to implement in a good way.

  • Dyanmic handle for rows and cols. It should work for anything over 2 rows and 2 cols.



  • Victory check and click tracker.



GUI.java

```
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

import java.util.ArrayList;
import java.util.List;

public class GUI implements ActionListener
{
public static final int WINDOW_MIN_X = 240;
public static final int WINDOW_MIN_Y = 200;

public static final int COLS = 4;
public static final int ROWS = 4;
public static final int HGAP = 3;
public static final int VGAP = 3;

public static final int NUMBER_OF_CELLS = ROWS * COLS;

private JFrame frame;
private List buttons = new ArrayList<>();
private List correct = new ArrayList<>();
private Cell emptyButton;

private int movesCounter;

/**
* Constructor for objects of class GUI
*/
public GUI ()
{
frame = new JFrame("Slide Game");

frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);

frame.setMinimumSize(new Dimension (WINDOW_MIN_X, WINDOW_MIN_Y));

frame.setLayout (new GridLayout (ROWS, COLS, HGAP, VGAP));

for( Integer i = NUMBER_OF_CELLS-1; i >= 0 ; i-- ) {
createCell (i);
}

emptyButton = buttons.get (NUMBER_OF_CELLS-1);

frame.setVisible(true);
frame.pack();

setVictoryCondition ();
}

public void actionPerformed (ActionEvent e) {
if (checkNeighbour((Cell)e.getSource())) {
swap ((Cell)e.getSource());
movesCounter++;
ch

Solution

Cell class

public class Cell extends JButton


I would not extend JButton. It is better to use composition/encapsulation and store a JButton inside the Cell class. Right now you can call plenty of JButton methods from outside the Cell class that is totally irrelevant to the Cell itself. Just check the many inherited methods that a JButton has!

if (!(buttons.get(i).getText ().equals( correct.get(i).getText ())))


No need to wrap it in an extra set of parenthesis, and please watch your spacing.

if (!buttons.get(i).getText().equals(correct.get(i).getText()))


However, when you apply the advice above and encapsulate the JButton inside the Cell class, you can use something like this in your Cell class:

int getNumber() {
    return this.number;
}

void setNumber(int number) {
    this.number = number;
    this.button.setText(number == 0 ? " " : String.valueOf(number));
}


If you are unsure about that a ? b : c syntax. You can also use an if-else statement:

if (number == 0) {
        this.button.setText(" ");
    } else {
        this.button.setText(String.valueOf(number));
    }


public Cell (Integer value)


You are using Integer instead of int in your entire program. You can use a primitive int instead, which is preferred.

row = GUI.NUMBER_OF_CELLS / GUI.COLS-1 - ( value / GUI.COLS);
col = GUI.NUMBER_OF_CELLS-1 - (row*GUI.COLS + value);


You are making your Cell dependent on your GUI here. This is generally a bad idea. I would instead pass the row and col to the Cell as constructor arguments.
GUI class

if (!(x == 0 && y == 1 || y == 0 && x == 1)) {
    return false;
}


"If not x equals zero and y equals one or y equals 0 and x equals one." Please don't have me read that again.

I would add extra parenthesis around x == 0 && y == 1 to avoid mixing up && and ||.

Also, by switching the logic around it becomes this, which is clearer:

if ((x == 0 && y == 1) || (y == 0 && x == 1)) {
    return true;
}
return false;


This can be rewritten to:

return (x == 0 && y == 1) || (y == 0 && x == 1);


Which is the same as:

return x + y == 1;


Ain't some math beautiful? :)

c.addActionListener(
    (ActionEvent e) -> {actionPerformed(e);}
);


+1 for using Lambdas! You can write this as:

c.addActionListener(this::actionPerformed);

Code Snippets

public class Cell extends JButton
if (!(buttons.get(i).getText ().equals( correct.get(i).getText ())))
if (!buttons.get(i).getText().equals(correct.get(i).getText()))
int getNumber() {
    return this.number;
}

void setNumber(int number) {
    this.number = number;
    this.button.setText(number == 0 ? " " : String.valueOf(number));
}
if (number == 0) {
        this.button.setText(" ");
    } else {
        this.button.setText(String.valueOf(number));
    }

Context

StackExchange Code Review Q#78501, answer score: 6

Revisions (0)

No revisions yet.