I'm trying to make a first game. I would like to make memory game where you have to find a color matched field.

I made:

MemoryGame - JFrame where I added mainPanel.
In mainPanel I added two another panel becouse both have other layout.
The most of the game code is in ButtonsPanel class. Code in this class work almost like I want.

I don't know how to write the BottomPanel class. I wish to make a restart button which will clear all buttons and will start the game from begining.
I wish to make two JLablel, one of this will show messages like "this field is already matched" etc. and second one will show a number of showed fields.

Can you check this code and give me some comments and leads?

Maybe I should move most of the code from buttomsPanel to MainPanel.

Card Class:

package memoryGame;
import javax.swing.*;

public class Card extends JButton 

private boolean cardMatched;
private boolean cardSelected;

private String back;


public Card(String back)

 this.back = back;
 cardSelected = false;
 cardMatched = false;


public String getBack() 
 return back;


public void setMatched(boolean matched) 
 cardMatched = matched;


public void setSelected(boolean selected) 
 cardSelected = selected;


public boolean isCMatched() 
 if (cardMatched == true) 
 return true;
 else 
 return false;
 


public boolean isCSelected() 
 if (cardSelected == true) 
 return true;
 else 
 return false;
 

ButtonsPanel Class - with a most of the game code:

package memoryGame;
import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.lang.reflect.Field;
import java.util.Random;
import static java.lang.String.valueOf;

public class ButtonsPanel extends JPanel implements ActionListener

private static final int NUMBER_OF_ROWS = 4;
private static final int NUMBER_OF_COLUMNS = 4;
private static final int HORIZONTAL_GAP = 5;
private static final int VERTICAL_GAP = 5;
private static final int PANEL_BORDER = 20;


private Card cards = null;
private String colors = "Blue", "Blue", "Red", "Red", "Green", "Green", "Magenta", "Magenta", "Orange", "Orange", "Cyan", "Cyan", "Black", "Black", "Pink", "Pink";

private Random random;
private Field field;
private Color color;
private int score = 0;
private Card selectedCard;
private Card c1;
private Card c2;
private Card c3;

public ButtonsPanel()
 setBackground(Color.BLACK);

 GridLayout layout = new GridLayout(NUMBER_OF_ROWS, NUMBER_OF_COLUMNS, HORIZONTAL_GAP, VERTICAL_GAP);
 setLayout(layout);

 setBorder(BorderFactory.createEmptyBorder(PANEL_BORDER,PANEL_BORDER,PANEL_BORDER,PANEL_BORDER));

 cards = new Card[NUMBER_OF_ROWS][NUMBER_OF_COLUMNS];
 shuffle();
 setCards();


public void setCards() 
 int a = 0;
 for (int row = 0; row < cards.length; row++) 
 for (int column = 0; column < cards[0].length; column++) 
 cards[row][column] = new Card(colors[a]);
 add(cards[row][column]);
 cards[row][column].addActionListener(this);
 a++;
 
 


public void shuffle() 
 random = new Random();
 for (int i = 0; i < colors.length; i++) 
 int pos = random.nextInt(colors.length);
 String temp = colors[i];
 colors[i] = colors[pos];
 colors[pos] = temp;
 


public void setColors(Card card) 
 try 
 field = Class.forName("java.awt.Color").getField(card.getBack().toUpperCase());
 color = (Color) field.get(null);
 card.setBackground(color);
 catch (Exception ex) 
 color = null;
 


public void showCardColor() 
 for (int row = 0; row < cards.length; row++) 
 for (int column = 0; column < cards[0].length; column++) 
 if (selectedCard == cards[row][column]) 
 if (!cards[row][column].isCMatched()) 
 if (!cards[row][column].isCSelected()) 
 setColors(cards[row][column]);
 cards[row][column].setSelected(true);
 mouseClick++;
 System.out.println(mouseClick);
 else 
 System.out.println("This field is already selected");
 
 else 
 System.out.println("This file is already matched.");
 
 
 
 



int mouseClick = 0;

public void hideColor() 
 if (mouseClick > 1 && mouseClick % 2 != 0) 
 c1.setBackground(new JButton().getBackground());
 c2.setBackground(new JButton().getBackground());
 c1 = null;
 c2 = null;
 


public void check() 
 if (c1.getBack().equals(c2.getBack())) 
 c1.setMatched(true);
 c2.setMatched(true);
 if (isEndOfGame() == true) 
 JOptionPane.showMessageDialog(this, "You won in " + score + " moves !");
 
 c1 = null;
 c2 = null;
 c3 = null;

 c1 = selectedCard;
 showCardColor();

 else 
 showCardColor();
 c1.setSelected(false);
 c2.setSelected(false);
 hideColor();
 c1 = c3;
 c3 = null;
 


public boolean isEndOfGame() 

 for (Card cards2d : cards) 
 for (Card cards1d : cards2d) 
 if (cards1d.isCMatched() == false) 
 return false;
 
 
 
 return true;


public void doTurn() 
 if (c1 == null && c2 == null) 
 if (!selectedCard.isCMatched()) 
 c1 = selectedCard;
 showCardColor();
 
 
 if (c1 != null && c1 != selectedCard && c2 == null) 
 if (!selectedCard.isCMatched()) 
 c2 = selectedCard;
 showCardColor();

 
 
 if (c1 != null && c2 != null && c2 != selectedCard && c3 == null) 
 c3 = selectedCard;
 check();
 


Object source;
@Override
public void actionPerformed(ActionEvent e) 
 source = e.getSource(); 

 selectedCard = (Card) source;

 doTurn();
 score++;

BottomPanel Class:

package memoryGame;
import javax.swing.*;
import java.awt.*;

public class BottomPanel extends JPanel 

private JButton buttonRestart;
private JLabel labelScore;
private JLabel labelMesseges;
private BorderLayout borderLayout;
int score = 0;

public BottomPanel()

 borderLayout = new BorderLayout();
 setLayout(borderLayout);
 buttonRestart = new JButton("Restart");
 buttonRestart.setPreferredSize(new Dimension(150,50));
 labelMesseges = new JLabel();
 labelScore = new JLabel("" + labelScore, SwingConstants.CENTER);
 labelScore.setPreferredSize(new Dimension(50,50));

 labelMesseges.setText(" ");
 labelMesseges.setBorder(BorderFactory.createMatteBorder(0,2,0,2,Color.black));

 labelScore.setFont(new Font("Courier", Font.BOLD, 16));
 labelScore.setText("36");

 add(buttonRestart);
 add(labelMesseges);
 add(labelScore);
 borderLayout.addLayoutComponent(buttonRestart, BorderLayout.WEST);
 borderLayout.addLayoutComponent(labelMesseges,BorderLayout.CENTER);
 borderLayout.addLayoutComponent(labelScore,BorderLayout.EAST);

MainPanel Class:

package memoryGame;

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

public class MainPanel extends JPanel

private BorderLayout borderLayout;
private BottomPanel bottomPanel;
private ButtonsPanel buttonsPanel;

public MainPanel()

 borderLayout = new BorderLayout(1,1);

 setLayout(borderLayout);

 buttonsPanel = new ButtonsPanel();
 bottomPanel = new BottomPanel();

 add(buttonsPanel);
 add(bottomPanel);
 borderLayout.addLayoutComponent(buttonsPanel, BorderLayout.CENTER);
 borderLayout.addLayoutComponent(bottomPanel, BorderLayout.SOUTH);

MemoryGame Class:

package memoryGame;

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

public class MemoryGame extends JFrame 

 private static final int WINDOW_WIDTH = 900;
 private static final int WINDOW_HEIGHT = 900;

 private MainPanel mainPanel;

 public MemoryGame() 

 setSize(new Dimension(WINDOW_WIDTH, WINDOW_HEIGHT));
 setLocationRelativeTo(null);
 setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
 setResizable(true);
 setTitle("Memory Game");

 mainPanel = new MainPanel();

 add(mainPanel);

 setVisible(true);
 

Test Class:

package memoryGame;

public class Test

 public static void main(String args) throws Exception 

 new MemoryGame();

 

Thank for sharing your code.

Pros

• You actually follow the
  Java Naming Conventions
  


• you created a package
  


• you created constants for "magic numbers".

Cons

- OOP

OOP doesn't mean to "split up" code into random classes.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (among others):

• information hiding / encapsulation


• single responsibility


• separation of concerns


• KISS (Keep it simple (and) stupid.)


• DRY (Don't repeat yourself.)


• "Tell! Don't ask."


• Law of demeter ("Don't talk to strangers!")


• replace branching with polymorphism

Inheritance

We extend existing classes if we need to chaing the behavior of the base class, that is: we overwrite one of the base classes methods to do something additional and/or different.

Your classes extend JPanel and JFrame but do not change their behavior, you just configure them which you could also do "from outside"

Separations of Concerns / Single Responsibility

Your classes mix business logic with user interaction. This should always be separated. The best way to do this is to follow the Model View Controller Pattern.

- Coding practices

limit scope of variables

in class ButtonsPanel you have a variable source defined as member variable but you only access it inside the method actionPerformed(). Therefore it should be defined as local variable inside that method.

do not have top level class implement *Listener interfaces

Swings *listener interfaces are meant to be implemented by small anonymous inner classes. Do not let you top level classes (the one that defines the file name) this type of interface (unless it is not having any other responsibility).

But if i need one Panel with a gridLayout and second with borderLayout i have to do two separated panels. I still dont know how to do it if i need to change value in bottomPanel, JLabel labelscore every time if someone click a field in buttonsPanel.

here is a quick and dirty alternative implementation demonstrating:

• configuring Swing components from the outside rather than in a child classes constructor,


• use of anonymous inner classes (AbstractAction)


• separation of different task into methods

Please keep in mind that this example is far from being perfect...

package stackexchange.codereview;

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;

import javax.swing.AbstractAction;
import javax.swing.JButton;
import javax.swing.JComponent;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JPanel;

public class ConnectedButtons 

 private static final int APPLICATION_SIZE = 400;
 private static final Color BACKGROUND = new JLabel().getBackground();

 private int score = 0;

 public static void main(String args) 
 new ConnectedButtons().runGame();
 

 private void runGame() 
 JFrame application = new JFrame("color game");
 JLabel scoreLabel = new JLabel("score: "
 + score);
 List<Color> colors = Arrays.asList(Color.BLUE, Color.CYAN, Color.RED, Color.YELLOW, Color.GREEN, Color.BLACK);
 List<JButton> fieldCells = initializeGame(colors);
 JPanel gameFiled = initializeView(fieldCells);
 bindViewToModel(colors, fieldCells, scoreLabel);
 JPanel gameControl = setupController(colors, fieldCells, application, scoreLabel);
 application.getContentPane().add(gameFiled);
 application.getContentPane().add(gameControl, BorderLayout.SOUTH);
 application.setSize(APPLICATION_SIZE, 400);
 application.setVisible(true);
 

 private JPanel setupController(List<Color> colors,
 List<JButton> fieldCells,
 JFrame application,
 JLabel scoreLabel) 
 JPanel gameControl = new JPanel(new GridLayout(1, 0));
 gameControl.add(new JButton(new AbstractAction("restart") 
 @Override
 public void actionPerformed(ActionEvent e) 
 bindViewToModel(colors, fieldCells, scoreLabel);
 
 ));
 gameControl.add(scoreLabel);
 gameControl.add(new JButton(new AbstractAction("quit") 
 @Override
 public void actionPerformed(ActionEvent e) 
 application.dispose();
 
 ));
 return gameControl;
 

 private void bindViewToModel(List<Color> colors, List<JButton> fieldCells, JLabel scoreLabel) 
 Collection<JComponent> clickedButtons = new HashSet<>(); // Model
 Collections.shuffle(fieldCells);
 Iterator<JButton> randomCells = fieldCells.iterator();
 for (Color color : colors) 
 AbstractAction buttonAction = createButonAction(clickedButtons, color, scoreLabel);
 bindButton(buttonAction, randomCells.next());
 bindButton(buttonAction, randomCells.next());
 
 clickedButtons.clear();
 score = 0;
 

 private void bindButton(AbstractAction buttonAction, JButton jButton) 
 jButton.setAction(buttonAction);
 jButton.setBackground(BACKGROUND);
 

 private JPanel initializeView(List<JButton> fieldCells) 
 JPanel gameFiled = new JPanel(new GridLayout(4, 0));
 for (JButton fieldCell : fieldCells) 
 fieldCell.setBackground(BACKGROUND);
 fieldCell.setEnabled(true);
 gameFiled.add(fieldCell);
 
 return gameFiled;
 

 private List<JButton> initializeGame(Collection<Color> colors) 
 List<JButton> fieldCells = new ArrayList<>();
 for (Color color : colors) 
 fieldCells.add(new JButton()); // two buttons per color
 fieldCells.add(new JButton());
 
 return fieldCells;
 

 private AbstractAction createButonAction(Collection<JComponent> clickedButtons, Color color, JLabel scoreLabel) 
 AbstractAction buttonAction = new AbstractAction() // Controller
 Collection<JComponent> clickedPartners = new HashSet<>(); // also Model

 @Override
 public void actionPerformed(ActionEvent e) 
 JComponent thisButton = (JComponent) e.getSource();
 clickedPartners.add(thisButton);
 clickedButtons.add(thisButton);
 thisButton.setBackground(color);
 thisButton.setEnabled(false);
 if (2 == clickedButtons.size()) // is second clicked
 if (2 == clickedPartners.size()) // user found partner
 score += 10;
 else 
 JOptionPane.showMessageDialog(thisButton, "no match");
 for (JComponent partner : clickedButtons) 
 partner.setBackground(BACKGROUND);
 partner.setEnabled(true);
 
 score--;
 
 clickedButtons.clear();
 clickedPartners.clear();
 scoreLabel.setText("score: "
 + score);
 
 
 ;
 return buttonAction;