Breaking memory game code into MVC form

Question

I am currently coding a memory game whereby upon the game starts with 14 cards displayed faced down.

• The player will start by clicking on 2 cards.
• The cards will be revealed.
• If both of the card ranks does not match,both cards will be faced down again after 2 second and it will be dealer (BOT) turn.
• The dealer (BOT) will randomly choose two cards.
• The cards will be revealed.
• If both of the card ranks does not match, both cards will be faced down again after 2 second and it will be the player turn again.
• This goes on until the the deck and table has no more cards.

I only coded until the part whereby the it will be the dealer's turn when the player chosen cards does not match versa.

I tried to separate my code into MVC form and I don't know whether I am doing it correctly.

Also in my controller class, after the player has chose 2 cards and did not match, I initialized a thread to make the program do nothing before showing the cards faced down and and go to dealers turn. I also have another thread to make the program do nothing before going to player's turn after the dealer chose the both cards that does not match. I feel that I am doing it in a procedural way and I want to improve on it if possible as well as my MVC for my code.

This is all my models

CardRank.java

public enum CardRank {
        Ace,
        Two,
        Three,
        Four,
        Five,
        Six,
        Seven,
        Eight,
        Nine,
        Ten,
        Jack,
        Queen,
        King;
}

CardSuits.java

public enum CardSuits {
      Diamonds,
      Clubs,
      Hearts,
      Spades;
}

Card.java

import java.awt.BorderLayout;
import java.util.ArrayList;
import javax.swing.Icon;
import javax.swing.ImageIcon;
import javax.swing.JLabel;
import javax.swing.JPanel;

@SuppressWarnings("serial") 
public class Card extends JPanel {

    private CardSuits suits;
    private CardRank rank;
    private int cardPos;

    public Card() {

    }

    public Card(CardSuits suits, CardRank rank) {
            setCardSuit(suits);
            setCardRank(rank);
    }

    public Card(int cardPos, CardRank rank) {
            setCardPos(cardPos);
            setCardRank(rank);
    }

    public CardSuits getCardSuit() {
            return suits;
    }
    public CardRank getCardRank() {
            return rank;
    }

    public int getCardPos() {
            return cardPos;
    }

    public void setCardSuit(CardSuits suits) {
            this.suits = suits;
    }
    public void setCardRank(CardRank rank) {
            this.rank = rank;
    }
    public void setCardPos(int cardPos) {
            this.cardPos = cardPos;
    }

    public String toString() {
            String cardType;
            cardType = suits + " of " + rank;
            return cardType;
    }
}

Deck.java

import java.util.ArrayList;
import java.util.Collections;

public class Deck {
    private ArrayList<Card> deck = new ArrayList<Card>();

    Card card = new Card();
    public Deck() {
            deckOfCards();
    }

    public void deckOfCards() {
            for(int i = 0; i < 2; i++) {
                    for(int j = 0; j < 13; j++) {
                            deck.add(new Card(CardSuits.values()[i],CardRank.values()[j]));
                    }
            }
    }

    public void shuffle() {
            shuffle(deck);
    }

    public Card dealCard() {
            return deck.remove(0);
    }

    private void shuffle(ArrayList<Card> deck) {
            Collections.shuffle(deck);
            for(int i = 0; i < deck.size(); i++) {
                    System.out.println(deck.get(i).toString());
            }
    }

    public static void main(String []args) {
            Deck deck = new Deck();
            deck.shuffle();  
            System.out.println("===========");
            System.out.println(deck.dealCard());
    }
}

Dealer.java

import java.awt.Image;
import java.awt.image.BufferedImage;
import java.io.File;
import java.util.ArrayList;
import java.util.Random;
import javax.imageio.ImageIO;

public class Dealer {

    Deck deck = new Deck();
    private ArrayList<Card> cardsOnTableL = new ArrayList<Card>();
    private int cardPosOne = 0 , cardPosTwo = 0, currTopCard = 0;
    String dCardOneSuit = " ", dCardOneRank = " ", dealerFirstCard  = " ";
    String dCardTwoSuit = " ", dCardTwoRank = " ", dCardTwoCardType  = " ";

    public Dealer() {

            deck = new Deck();
    }

    public void shuffleCards(){    
            deck.shuffle();
    }

    public ArrayList<Card> dealFirst14Card() {
            for(int i = currTopCard; i < 14; i++) {
                    cardsOnTableL.add(deck.dealCard());
            }
            return cardsOnTableL;
    }


    public Image hideCard() {
            BufferedImage img = null;
            try {
                    img = ImageIO.read(new File("resources/images/blank.png"));      
            } catch (Exception e) {
            }
            return img;
    }

    public  Image revealCard(String cardType) {
            BufferedImage img = null;
            try {
                    img = ImageIO.read(new File("resources/images/"+cardType+".png"));       
            } catch (Exception e) {
            }

            return img;
    }

    public Image placeBlankCard() {
            BufferedImage img = null;
            try {
                    img = ImageIO.read(new File("resources/images/dotframe.png"));   
            } catch (Exception e) {
            }

            return img;
    }

    public int[] randomNumber() {
            Random r = new Random();
            do {
                    cardPosOne = r.nextInt(13);
                    cardPosTwo = r.nextInt(13);
            }while((cardPosOne == cardPosTwo));
             return new int[] {cardPosOne, cardPosTwo};
    }

    public String firstCardSuit(ArrayList<Card> cardsDealted, int cardPosOne) {
            dCardOneSuit = cardsDealted.get(cardPosOne).getCardSuit().name();
            return dCardOneSuit;
    }

    public String firstCardRank(ArrayList<Card> cardsDealted, int cardPosOne) {
            dCardOneRank = cardsDealted.get(cardPosOne).getCardRank().name();
            return dCardOneRank;
    }

    public String chooseFirstCard(String dCardOneSuit, String dCardOneRank) {
            dealerFirstCard = dCardOneSuit + dCardOneRank;
            return dealerFirstCard;
    }

    public String secondCardSuit(ArrayList<Card> cardsDealted, int cardPosTwo) {
            dCardTwoSuit = cardsDealted.get(cardPosTwo).getCardSuit().name();
            return dCardTwoSuit;
    }

    public String secondCardRank(ArrayList<Card> cardsDealted, int cardPosTwo) {
            dCardTwoRank = cardsDealted.get(cardPosTwo).getCardRank().name();
            return dCardTwoRank;
    }

    public String chooseSecondCard(String dCardTwoSuit, String dCardTwoRank) {
            dCardTwoCardType = dCardTwoSuit + dCardTwoRank;
            return dCardTwoCardType;
    }


    public void dealNextTwoCards(int cardPosOne, int cardPosTwo) {
            cardsOnTableL.remove(cardPosOne);
            cardsOnTableL.add(cardPosOne,deck.dealCard());
            cardsOnTableL.remove(cardPosTwo);
            cardsOnTableL.add(cardPosTwo,deck.dealCard());

    }
}

Player.java

import java.util.ArrayList;
import javax.swing.JLabel;

public class Player {
    private String playerName;
    private int firstCardPos,secondCardPos;
    private boolean isSelected = false;
    private String playerFirstCard = " ",pCardOneSuit = " ", pCardOneRank = " ";
    private String playerSecondCard = " ",pCardTwoSuit = " ", pCardTwoRank = " ";

    public Player() {

    }

    public String getPlayerName() {
            return playerName;
    }

    public void setPlayerName(String playerName) {
            this.playerName = playerName;
    }

    public int firstCardPos(JLabel label, ArrayList<Card> cardsDealt, JLabel[] cardsOnTable) {
            for(int i = 0; i< cardsDealt.size(); i++) {
                    if(label == cardsOnTable[i]) {
                            firstCardPos = i;
                    }
            }
            return firstCardPos;
    }

    public String firstCardSuit(ArrayList<Card> cardsDealted, int cardPosOne) {
            pCardOneSuit = cardsDealted.get(cardPosOne).getCardSuit().name();
            return pCardOneSuit;
    }

    public String firstCardRank(ArrayList<Card> cardsDealted, int cardPosOne) {
            pCardOneRank = cardsDealted.get(cardPosOne).getCardRank().name();
            return pCardOneRank;
    }

    public String chooseFirstCard(String pCardOneSuit, String pCardOneRank) {
            playerFirstCard = pCardOneSuit + pCardOneRank;
            return playerFirstCard;
    }

    public String secondCardSuit(ArrayList<Card> cardsDealted, int cardPosTwo) {
            pCardTwoSuit = cardsDealted.get(cardPosTwo).getCardSuit().name();
            return pCardTwoSuit;
    }

    public String secondCardRank(ArrayList<Card> cardsDealted, int cardPosTwo) {
            pCardTwoRank = cardsDealted.get(cardPosTwo).getCardRank().name();
            return pCardTwoRank;
    }

    public String chooseSecondCard(String pCardTwoSuit, String pCardTwoRank) {
            playerSecondCard = pCardTwoSuit + pCardTwoRank;
            return playerSecondCard;
    }

    public int secondCardPos(JLabel label, ArrayList<Card> cardsDealt, JLabel[] cardsOnTable) {
            for(int i = 0; i< cardsDealt.size(); i++) {
                    if(label == cardsOnTable[i]) {
                            secondCardPos = i;
                    }
            }
            return secondCardPos;
    }

    public boolean disableClickFirstCard(JLabel label, ArrayList<Card> cardsDealt, JLabel[] cardsOnTable) {
            for(int i = 0; i< cardsDealt.size(); i++) {
                    if(label == cardsOnTable[i]) {
                            firstCardPos = i;
                            isSelected = true;
                    }
            }
            return isSelected;
    }
}

This is my view

GameTable.java

import java.awt.GridLayout;
import java.awt.event.ActionListener;
import java.awt.event.MouseListener;
import java.util.ArrayList;
import javax.swing.ImageIcon;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;

@SuppressWarnings("serial")
public class GameTable extends JFrame {

    private JLabel[] cardsOnTable = new JLabel[14];
    private ArrayList<Card> cardsDealted = new ArrayList<Card>();
    private JPanel cardsPanel = new JPanel(new GridLayout(2,7));
    private Dealer dealer = new Dealer();

    public GameTable() {
            //setLayout(new GridBagLayout());
            cardsDealted = dealer.dealFirst14Card();
            for(int i = 0; i < cardsDealted.size(); i++) {
                    cardsOnTable[i] = new JLabel(new ImageIcon(dealer.hideCard()));
                    cardsOnTable[i].setText(i + " ");
                    cardsOnTable[i].setHorizontalTextPosition(JLabel.CENTER);
                    cardsOnTable[i].setVerticalTextPosition(JLabel.TOP);
                    cardsPanel.add(cardsOnTable[i]);
                    add(cardsPanel);
                    pack();
                    setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
                    setSize(1024,768);
                    setLocationRelativeTo(null);
            }
    }

    public JLabel[] getJLabel() {
            return cardsOnTable;
    }

    public void setJLabel(JLabel[] cardsOnTable) {
            this.cardsOnTable = cardsOnTable;
    }

    public ArrayList<Card> getCardsDealted() {
            return cardsDealted;
    }

    public void setCardsDealted( ArrayList<Card> cardsDealted) {
            this.cardsDealted = cardsDealted;
    }

    void addMouseListenerForJLabel(MouseListener listenForMouseClick) {
            for(int i = 0; i < 14; i++) {
                    cardsOnTable[i].addMouseListener(listenForMouseClick);
            }      
    }
}

This is my controller

FishingGame.java

import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import javax.swing.ImageIcon;
import javax.swing.JLabel;

public class FishingGame {

    private GameTable gameTable;
    private Dealer dealer;
    private Player player;
    private int pCardPosOne = 0 , pCardPosTwo = 0 , clicked = 0;
    private boolean playerTurn = true;
    private String pCardOneSuit = " " , pCardOneRank = " ", playerFirstCard = " ";
    private String pCardTwoSuit = " " , pCardTwoRank = " ", playerSecondCard = " ";
    private String dCardOneSuit = " " , dCardOneRank = " ", dealerFirstCard = " ";
    private String dCardTwoSuit = " " , dCardTwoRank = " ", dealerSecondCard = " ";
    private int[] dealerChoice;

    public FishingGame(GameTable gameTablePanel,Dealer dealer,Player player) {
            this.gameTable = gameTablePanel;
            this.dealer = dealer;
            this.player = player;
            this.gameTable.addMouseListenerForJLabel(new MouseHandler());

    }

    class MouseHandler extends MouseAdapter {
            boolean isClickEnabled = true; 
            public void mousePressed(MouseEvent e) {
                    JLabel label = (JLabel) e.getSource();
                    if(!isClickEnabled) {
                            return;
                    }      
                    clicked++;
                    if(playerTurn) {
                            if(clicked == 1) {
                                    pCardPosOne = player.firstCardPos(label, gameTable.getCardsDealted(),  gameTable.getJLabel());
                                    pCardOneSuit = player.firstCardSuit(gameTable.getCardsDealted(), pCardPosOne);
                                    pCardOneRank = player.firstCardRank(gameTable.getCardsDealted(), pCardPosOne);
                                    playerFirstCard = player.chooseFirstCard(pCardOneSuit,pCardOneRank);
                                    label.setIcon(new ImageIcon(dealer.revealCard(playerFirstCard)));
                            }
                            else if(clicked == 2) {
                                    pCardPosTwo = player.secondCardPos(label, gameTable.getCardsDealted(),  gameTable.getJLabel());
                                    pCardTwoSuit = player.firstCardSuit(gameTable.getCardsDealted(), pCardPosTwo);
                                    pCardTwoRank = player.firstCardRank(gameTable.getCardsDealted(), pCardPosTwo);
                                    playerSecondCard = player.chooseFirstCard(pCardTwoSuit,pCardTwoRank);
                                    label.setIcon(new ImageIcon(dealer.revealCard(playerSecondCard)));
                                    if(pCardOneRank.equals(pCardTwoRank)) {
                                            System.out.println("Player caught a fish!");
                                    }

                                    else {
                                            System.out.println("Player missed the fish!");
                                            new Thread(new playerMissCardThread()).start();
                                    }

                            }
                    }              
            }
    }

    class playerMissCardThread implements Runnable {
            @Override
            public void run() {

                    try   {              
                            Thread.sleep(2000);            
                    }          
                    catch (InterruptedException interruptedException)          
                    {                          
                            System.out.println( "Thread is interrupted when it is sleeping" +interruptedException);            
                    }    
                    playerTurn = false;
                    if(!playerTurn) {
                            gameTable.getJLabel()[pCardPosOne].setIcon(new ImageIcon(dealer.hideCard()));
                            gameTable.getJLabel()[pCardPosTwo].setIcon(new ImageIcon(dealer.hideCard()));
                            dealerChoice = dealer.randomNumber();
                            dCardOneSuit = dealer.firstCardSuit(gameTable.getCardsDealted(), dealerChoice[0]);
                            dCardOneRank = dealer.firstCardRank(gameTable.getCardsDealted(), dealerChoice[0]);
                            dealerFirstCard  = dealer.chooseFirstCard(dCardOneSuit,dCardOneRank);

                            dCardTwoSuit = dealer.secondCardSuit(gameTable.getCardsDealted(), dealerChoice[1]);
                            dCardTwoRank = dealer.secondCardRank(gameTable.getCardsDealted(), dealerChoice[1]);
                            dealerSecondCard  = dealer.chooseSecondCard(dCardTwoSuit,dCardTwoRank);
                            gameTable.getJLabel()[dealerChoice[0]].setIcon(new ImageIcon(dealer.revealCard(dealerFirstCard)));
                            gameTable.getJLabel()[dealerChoice[1]].setIcon(new ImageIcon(dealer.revealCard(dealerSecondCard)));
                            if(dCardOneRank.equals(dCardTwoRank)) {
                                    System.out.println("Dealer caught a fish!");
                            }

                            else {
                                    System.out.println("Dealer missed the fish!");
                                    new Thread(new dealerMissCardThread()).start();
                            }
                    }
            }  
    }

    class dealerMissCardThread implements Runnable {
            @Override
            public void run() {
                    try   {              
                            Thread.sleep(2000);  
                            gameTable.getJLabel()[dealerChoice[0]].setIcon(new ImageIcon(dealer.hideCard()));
                            gameTable.getJLabel()[dealerChoice[1]].setIcon(new ImageIcon(dealer.hideCard()));
                            clicked = 0;
                            playerTurn = true;
                    }  

                    catch (InterruptedException interruptedException)          
                    {                          
                            System.out.println( "Thread is interrupted when it is sleeping" +interruptedException);            
                    }
            }
    }
}

main()

public class GameModule {
    public static void main(String[]args) {
        GameTable view = new GameTable();
        Dealer m1 = new Dealer();
        Player m2 = new Player();
        new FishingGame(view,m1,m2);
        view.setVisible(true);
    }
}

Answer 1

My basic guideline for MVC and similar patterns -- you know that you are on the right track when you can easily re-use the components you have with a different interface.

For instance, it should be easy to write a version that plays from the command line, rather than from windows. It should be easy to write a test that plays the game with no UI at all. It should be straight forward to collect the parts of different families of UIs into different packages.

On to specifics

Why CardRank, but CardSuits? It reads as though you aren't applying your naming convensions consistently. I would expect to see the singular spelling in both cases.

public class Card extends JPanel

That's almost certainly wrong. A command line version of the game is going to include the concept of a "card", right? It's just going to present the cards differently (as text, most likely).

public class Card extends JPanel {

    private CardSuits suits;
    private CardRank rank;
    private int cardPos;

Very suspicious: that the Card has a suit and a rank makes a lot of sense. But what does cardPos mean? That looks like it's going to be part of a mapping of cards to locations (a place on the table, a place in the deck, a layer in the ui), but it isn't an intrinsic property of cards, or the game... so it belongs somewhere else.

public Card() {

}

Does it make sense to have a Card with no rank or suit?

public class Deck {
    private ArrayList<Card> deck = new ArrayList<Card>();

    Card card = new Card();
    public Deck() {
        deckOfCards();
    }

    public void deckOfCards() {
        for(int i = 0; i < 2; i++) {
                for(int j = 0; j < 13; j++) {
                        deck.add(new Card(CardSuits.values()[i],CardRank.values()[j]));
                }
        }
    }

In deckOfCards, your loops are broken -- it looks like you never include spades in your decks. The real problem here: you used the wrong kind of loop construct. One of the advantages of enumerated types is that they have functions for easily identifying all of their values -- so you should use them:

    public void deckOfCards() {
        for (CardSuits suits : CardSuits.values()) {
            for (CardRank rank : CardRank.values()) {
                deck.add(new Card(suits, rank));
            }
        }
    }

It's also a bit weird that each copy of the deck needs to make it's own cards, when they all are the same really? Especially since dealCard removes cards from the deck, but has no function to put them back.

There's probably a factory class trying to come out, where the factory provides a copy of a deck to use, and maybe another factory that knows to give you a shuffled copy of a deck of cards.

 public class Deck {
    private List<Card> deck;

    public Deck(List<Card> deck) {
        this.deck = deck;
    }

Notice the change from ArrayList to List -- Deck doesn't care what kind of list it gets....

public class Dealer {

    Deck deck = new Deck();
    private ArrayList<Card> cardsOnTableL = new ArrayList<Card>();
    private int cardPosOne = 0 , cardPosTwo = 0, currTopCard = 0;
    String dCardOneSuit = " ", dCardOneRank = " ", dealerFirstCard  = " ";
    String dCardTwoSuit = " ", dCardTwoRank = " ", dCardTwoCardType  = " ";

    public Dealer() {

            deck = new Deck();
    }

Messy - this has the obvious bug that you are creating a deck that you never use.
The Dealer should probably be passed a deck -- or a factory for decks -- rather than creating one of his own.

cardsOnTableL should be spelled cardsOnTable, and should be a List rather than an ArrayList, and almost certainly shouldn't be there at all -- the dealer plays cards on the Table, so give him a Table.

This is important, because the Table is really a key element in the abstraction - it's the thing that you are showing to the human player. So there really need to be two pieces to it -- the model of the table, which knows which cards are where, which are face up, which are face down, and so on, and then a view of the table, which doesn't know anything about the state of the cards, but knows how to draw what it is told.

So this thing:

public Image placeBlankCard() {
    BufferedImage img = null;
    try {
           img = ImageIO.read(new File("resources/images/dotframe.png"));   
    } catch (Exception e) {
    }

    return img;
}

is purely a view operation - it's a specific way of presenting the card to the user (but you could do it with text instead, right?). So it doesn't belong anywhere near the model -- it listens to the model. The dealer puts a card face down on the table, the table announces "hey everybody, there's a face down card at this location now", and this table view responds by drawing a blank card at that location on the screen. (The text only view is listening to the same thing, and draws a couple X's or something).

Also - don't throw away exceptions. If you are sure it can never happen, then do something like this:

    catch (Exception e) {
        throw new RuntimeException("The programmer promised this would never happen", e);
    }

which will cause your broken program to crash in a way you can diagnose, rather than doing something random and hiding the problem.