Two-player match-picking game

Question

I'm looking for some feedback on my first program. It's a small game you play with matches.

It's a two-player game, where the players take turns picking matches (1, 2 or 3) and cannot pick the same number of matches as the other player: if player one picks 2 matches, player two will have to pick either 1 or 3 matches. The player who cannot pick anymore matches loses the game, which means there are two end scenarios : when there are no more matches, or there is 1 but the other player picked 1 during the previous round.

Is there a more efficient way to do something (performance-wise or with less code to write)? Can I improve the overall presentation/organization of the code? (am I unaware of a convention?) Any advice or criticism is very welcome.

import java.util.Scanner ;


public class JeuDeNim2 {

public static void main(String[] args) {

    Scanner sc = new Scanner(System.in) ;

    int totalMatches ;
    do {
        System.out.println("How many matches do you want to play with? "
        + "(from 6 to 60)") ;
        totalMatches = sc.nextInt() ;
    }
    while (totalMatches < 6 || totalMatches > 60) ;


    int matchesThisTurn ;
    int matchesPreviousTurn = 0 ;
    int round = 0 ;
    int player = 0 ;
    int previousPlayer ;

    while (true) {

        round++ ;
        previousPlayer = player ;
        player = round % 2 ;
        if (player == 0) {
            player = 2 ;
        }

        while (true) {

            if (totalMatches == 1) {
                System.out.println("There is only one match left on "
                + "the table") ;
            } else {
                System.out.println("There are " + totalMatches
                + " matches on the table") ;
            }

            if (round == 1 || round == 2) {
                System.out.println("Player " + player + ": How many "
                + "matches do you want to pick? (1, 2 or 3)") ;
            } else {
                System.out.println("Player " + player + ": How many "
                + "matches do you want to pick this turn?") ;
            }

            matchesThisTurn = sc.nextInt() ;

            if (matchesThisTurn < 1 || matchesThisTurn > 3) {
                System.out.println("Wrong entry: you have to pick 1, 2 "
                + "or 3 matches") ;
                continue ;
            }
            if (matchesThisTurn == matchesPreviousTurn) {
                System.out.println("You cannot pick the same number of "
                + "matches as Player " + previousPlayer) ;
                continue ;
            }
            if (totalMatches == 1 && matchesThisTurn > totalMatches) {
                System.out.println("You cannot pick " + matchesThisTurn
                + " matches: there is only one match left") ;
                continue ;
            }
            if (matchesThisTurn > totalMatches) {
                System.out.println("You cannot pick " + matchesThisTurn
                + " matches: there are only " + totalMatches
                + " matches left") ;
                continue ;
            }
            break ;
        }

        totalMatches -= matchesThisTurn ;
        matchesPreviousTurn = matchesThisTurn ;

        if (totalMatches == 0) {
            System.out.println("*** There are no more matches! ***") ;
            break ;
        }
        if (totalMatches == 1 && matchesThisTurn == 1) {
            System.out.println("*** There is only one match left, but "
            + "Player " + player + " already took one! ***") ;
            break ;
        }
    }

    System.out.println("*** The game is over! Player " + player
    + " Wins! ***") ;
}
}

Answer 1

Java is an Object Oriented language... and I recognize that objects are not always the right solution to a problem.... The generally accepted "opposites" of object-orientation, though, are "procedural", and "functional". It is common (for example, in C) to write code as a collection of procedures, or (in haskell) to write it as functions.

In almost no languages, though, is it common to have no procedures, no objects, and no functions... well, except shell scripts, I guess. Your program has no methods, no objects, no functions, except the main method.

You need to break your code down in to parts that do logically isolated things, and then call those reusable chunks when needed.

Function 1

The first function I would extract, is a start-of-game match-count:

private static int getStartCount(Scanner sc) {
    while(true) {
        System.out.println("How many matches do you want to play with? "
                + "(from 6 to 60)") ;
        int totalMatches = sc.nextInt() ;
        if (totalMatches >= 6 && totalMatches <= 60) {
            return totalMatches;
        }
    }
}

Note, in that function I converted it from a do-while loop, to an infinite while-loop, with an early-return if the input is valid. I have nothing against do-while loops, but I find this early-return system simpler in terms of variable management. Note that the int totalMatches has a smaller scope than your code. You can call the code with:

int totalMatches = getStartCount(sc);

Function 2 (and 3)

Right, here's your input issue.... you have a big while loop, that plays each round, and inside that it has a user-input loop. The conditions on that loop are... confusing. What you have, is a prompt, some validation, and if everything is OK, you break. If there's a problem, you loop again. Again, extracting a function with an early return value, would be useful.

As an aside, when it comes to user input, it is almost always better to inform the user what input would be valid before requesting the input. Telling them they made a mistake afterwards is great, but telling them what their options are before, is better. Consider these two functions, the first function computes what would be a valid input, the second function uses that information to prompt the user:

private static int[] computeAllow(int totalMatches, int previous) {
    if (totalMatches == 1) {
        return new int[]{1};
    }
    if (totalMatches == 2 && previous < 3 && previous > 0) {
        return previous == 1 ? new int[]{2} : new int[]{1};
    }
    switch (previous) {
        case 0:
            return new int[]{1,2,3};
        case 1:
            return new int[]{2,3};
        case 2:
            return new int[]{1,3};
        case 3:
            return new int[]{1,2};

    }
    throw new IllegalStateException("Unexpected previous count " + previous);
}

Then, use this function like:

private static int getPlayerPick(Scanner sc, int totalMatches, int player, int previous) {
    if (totalMatches == 1) {
        System.out.println("There is only one match left on "
                + "the table") ;
    } else {
        System.out.println("There are " + totalMatches
                + " matches on the table") ;
    }

    final int[] allow = computeAllow(totalMatches, previous); 

    while (true) {

        System.out.printf("Player %d: How many matches do you want to pick? %s",
                player, Arrays.toString(allow));

        final int matchesThisTurn = sc.nextInt();
        for (int a : allow) {
            if (a == matchesThisTurn) {
                // Valid input.
                return matchesThisTurn;
            }
        }

        System.out.printf("Wrong entry: there are %d matches, "
            + "the last player selected %d, "
            + "which means you can only select one of %s\n",
                totalMatches, previous, Arrays.toString(allow));
    }
}

Main

Puting this all together, the main method becomes much simpler:

public static void main(String[] args) {

    try (Scanner sc = new Scanner(System.in) ;) {

        int totalMatches = getStartCount(sc);

        int matchesPreviousTurn = 0 ;
        int round = 0 ;
        int player = 0 ;

        do {

            //previousPlayer = player ;
            player = 1 + (round % 2);
            round++ ;

            int matchesThisTurn = getPlayerPick(sc, totalMatches, player, matchesPreviousTurn);

            totalMatches -= matchesThisTurn ;
            matchesPreviousTurn = matchesThisTurn ;

        } while (totalMatches > 1 || totalMatches == 1 && matchesPreviousTurn != 1);

        System.out.println("*** The game is over! Player " + player
                + " Wins! ***") ;
    }
}

It is still probably too busy, but you can at least see what's going on now.

Note how the user is prompted with valid values before they are entered now:

How many matches do you want to play with? (from 6 to 60)
10
There are 10 matches on the table
Player 1: How many matches do you want to pick? [1, 2, 3]
4
Wrong entry: there are 10 matches, the last player selected 0, which means you can only select one of [1, 2, 3]
Player 1: How many matches do you want to pick? [1, 2, 3]
2
There are 8 matches on the table
Player 2: How many matches do you want to pick? [1, 3]
2
Wrong entry: there are 8 matches, the last player selected 2, which means you can only select one of [1, 3]
Player 2: How many matches do you want to pick? [1, 3]
3
There are 5 matches on the table
Player 1: How many matches do you want to pick? [1, 2]
3
Wrong entry: there are 5 matches, the last player selected 3, which means you can only select one of [1, 2]
Player 1: How many matches do you want to pick? [1, 2]
2
There are 3 matches on the table
Player 2: How many matches do you want to pick? [1, 3]
1
There are 2 matches on the table
Player 1: How many matches do you want to pick? [2]
2
*** The game is over! Player 1 Wins! ***