Return to Revisions

Review:

public void shuffleCards()

Should only shuffle cards, because that's what it says it does. Create new methods void playGame() int getFirstPick(), int getSecondPick(), boolean isAlreadyMatched(int card1, int card2), boolean areAllCardsMatched() and possibly others.

Assign these methods to the class that they should belong to.

Game
    void playGame()        
    int getFirstPick()
    int getSecondPick()
    boolean verifyFirstPick(int firstPick)
    boolean verifySecondPick(int firstPick, int secondPick)

Cards
    void shuffleCards()
    boolean isAlreadyMatched(int card)
    boolean areAllCardsMatched()

Why?

Because right now you've got one long piece of code that does everything, and it's confusing you and causing bugs. By splitting it up, it'll be easier to identify what you need to do and whether the code is doing it.

If you do it right, you'll end up with code like this:

public void playGame(){
    //printInstructions(); //an idea, perhaps?
    setupGame();
    while(!isGameWon()){
        doTurn();
    }
    printGameWon();
}

That's awfully abstract, Pim.

Okay, lets dive a level deeper, in setupGame:

public void setupGame(){
    cards = new Cards();
    cards.shuffle();
    userInput = new Scanner(System.in);
}

That sets up the playing field and the input, it seems.

What's the next method? isGameWon()...

public boolean isGameWon(){
    return (cards != null && cards.areAllCardsMatched());
}

It's just a check for whether all the cards are matched.

Okay, what about doTurn() then? That one is a bit bigger.

public void doTurn(){
    int firstPick = -1;        
    do {
        firstPick = getFirstPick();
    } while(!verifyFirstPick(firstPick));
    printFirstPick(firstPick);

    int secondPick = -1;
    do {
        secondPick = getSecondPick();
    } while(!verifySecondPick(firstPick, secondPick));
    printSecondPick(secondPick);

    checkMatch(firstPick, secondPick);
}

It handles getting the first picked card and the second picked card, then passes them along to checking for a match.

Let's go look at the checkMatch function.

public void checkMatch(int firstPick, int secondPick){
    if(cards.match(firstPick, secondPick)){
        printMatch();
        printRemainingMatches();
    } else {
        printNoMatch();
    }
}

We STILL haven't seen any real code doing anything. All I have shown you right now that actually does anything is the setupGame method. The rest just structures the game flow.

Function lists right now:

Game
    void setupGame();
    void playGame();
    void doTurn();
    void checkMatch(int firstPick, int secondPick);     
    int getFirstPick();
    int getSecondPick();
    boolean verifyFirstPick(int firstPick);
    boolean verifySecondPick(int firstPick, int secondPick);
    boolean isGameWon();

    void printFirstPick(int firstPick);
    void printSecondPick(int secondPick);
    void printMatch();
    void printNoMatch();
    void printRemainingMatches();
    void printGameWon();

Cards
    void shuffleCards();
    boolean match(int card1, int card2);
    boolean isAlreadyMatched(int card);
    boolean areAllCardsMatched();

It'll be your task to implement the ones I haven't shown yet. You'll also have to move some of the variables so they're class members, not just declared in a function (hint: cards and userInput are two of these, I don't know if there are more).

Given the name of the functions, it should be easy to guess what they do.
shuffleCards shuffles the cards. match tests if two cards are a match. And so on...