13
\$\begingroup\$

I'm trying to design the classes for the Wheel of Fortune game in Java. The below diagram represents the classes and the interaction between them.

enter image description here. Below is the partial implementation of it. I appreciate some feedback on my design and approach. I'm mainly interested in classes and their dependencies. In code I referred MoneyPrize as DollarAmountSpinOutcome.

1) Is Wheel dependency on Player correct? If not how can I redesign to avoid it?

2) In method WheelOfFortune.getSpinOutcomeProcessor(SpinOutCome outcome) I am explicitly casting it to Specific spin out come. Is it a smell. Does it indicate a broken modeling?

public class WheelOfFortune {

    private List<Player> players;

    private Host host;

    private Wheel wheel;

    public WheelOfFortune(List<Player> players, Host host, Wheel wheel) {
        this.players = players;
        this.host = host;
        this.wheel = wheel;
    }

    public static void main(String[] args) {

        Player player1 = new Player("player1", new Account());
        Player player2 = new Player("player2",new Account());
        Player player3 = new Player("player3",new Account());
        ArrayList<Player> players = new ArrayList<Player>();
        players.add(player1);
        players.add(player2);
        players.add(player3);

        Host host = new Host();
        Wheel wheel = new Wheel();
        WheelOfFortune wheelOfFortune = new WheelOfFortune(players, host, wheel);
        wheelOfFortune.start();

    }

    public void start() {
        int round = 0;
        while (!isGameOver()) {
            round++;
            for (Player player : players) {

                boolean isTurnOver = false;
                while (!isTurnOver) {
                    Choice choice = player.makeChoice();
                    if (choice == Choice.SPIN_WHEEL) {
                        SpinOutCome spinOutCome = player.spinWheel(wheel, round);
                        isTurnOver = processSpinOutCome(spinOutCome);
                    } else if (choice == Choice.BUY_VOWEL) {
                        Character character = player.buyVowel();
                        isTurnOver = processOvalBuy(character);
                    }

                }
            }
        }
    }

    private static boolean processVowelBuy(Character character) {
        return false;
    }

    private boolean processSpinOutCome(SpinOutCome spinOutCome) {
        SpinOutComeProcessor spinOutComeProcessor = getSpinOutcomeProcessor(spinOutCome);
        return spinOutComeProcessor.process();
    }

    private SpinOutComeProcessor getSpinOutcomeProcessor(SpinOutCome spinOutCome) {

        if (spinOutCome instanceof DollarAmoutSpinOutcome)
            return new DollarAmoutSpinOutcomeProcessor(host, (DollarAmoutSpinOutcome) spinOutCome);
        else
            return null;

    }

    private static boolean isGameOver() {
        return false;
    }

}

Wheel

class Wheel {

        public SpinOutCome spin(Player player, int round) {

            // Return a random SpinOutcome;
            return new DollarAmoutSpinOutcome(player, 500, round);
        }
    }

DollarAmoutSpinOutCome

class DollarAmoutSpinOutcome extends SpinOutCome {

        private Integer dollarAmount;

        public DollarAmoutSpinOutcome(Player player, int dollarAmount, Integer round) {
            super(player, round);
            this.dollarAmount = dollarAmount;
        }

        public Integer getAmount() {
            return dollarAmount;
        }

    }

SpinOutCome

class SpinOutCome {

        protected Player player;

        protected Integer round;

        public SpinOutCome(Player player, Integer round) {
            this.player = player;
            this.round = round;
        }

        public Player getPlayer() {
            return player;
        }

        // DOLLAR_AMOUNT, LOSE_A_TURN, BANKRUPT, SOLVE_PUZZLE
    }

Choice

enum Choice {
        SPIN_WHEEL, BUY_VOWEL,
    }

Account

class Account {

        private Integer totalAmount;

        public void debit(Integer amount) {
        }

        public void credit(Integer ammount) {
        }

    }

Host

class Host {
        public List<Integer> getCharacterOccurences(Character c) {
            return null;
        }

        public void uncoverConsonent(List<Integer> characterOccurences) {
        }
    }

SpinOutComeLoseATurn

class SpinOutComeLoseATurn extends SpinOutCome {
        public SpinOutComeLoseATurn(Player player, Integer round) {
            super(player, round);
        }
    }

SpinOutComeProcessor

class SpinOutComeProcessor {

        private Host host;

        private Player player;

        public SpinOutComeProcessor(Host host, Player player) {
            this.host = host;
            this.player = player;
        }

        public Host getHost() {
            return host;
        }

        public Player getPlayer() {
            return player;
        }

        public boolean process() {
            return false;
        }
    }

Player

class Player {

            private Account account;

            private final String name;

            public Player(String name, Account account) {
                this.name = name;
                this.account = account;
            }

            public Choice makeChoice() {
                return null;
            }

            public Character guessConsonant() {
                return null;
            }

            public SpinOutCome spinWheel(Wheel wheel, int round) {
                return wheel.spin(this, round);
            }

            public String solvePuzzle() {
                return null;
            }

            public Character buyVowel() {
                return null;
            }

            public void acceptCashGift(Integer amount) {
                account.debit(amount);
            }

        }

DollarAmoutSpinOutcomeProcessor

class DollarAmoutSpinOutcomeProcessor extends SpinOutComeProcessor {

        private DollarAmoutSpinOutcome dollarSpinOutCome;

        public DollarAmoutSpinOutcomeProcessor(Host host, DollarAmoutSpinOutcome dollarSpinOutCome) {
            super(host, dollarSpinOutCome.getPlayer());
            this.dollarSpinOutCome = dollarSpinOutCome;
        }

        public boolean process() {
            boolean isTurnOver;
            Character character = getPlayer().guessConsonant();
            List<Integer> characterOccurences = getHost().getCharacterOccurences(character);
            if (characterOccurences.size() > 0) {
                getPlayer().acceptCashGift(dollarSpinOutCome.getAmount() * characterOccurences.size());
                getHost().uncoverConsonent(characterOccurences);
                isTurnOver = false;
            } else {
                isTurnOver = true;
            }
            return isTurnOver;
        }
    }
\$\endgroup\$
10
\$\begingroup\$

Thumbs up for the diagram! That's more by far compared to what most people do when starting such a project.

A few things I recognized at first sight:

  1. Player.buyOval() and WheelOfFortune.processOvalBuy(...) should read Vowel instead of Oval, shouldn't they?

  2. You have a SpinOutComeProcessor (that, according to the name, processes spin outcomes) and you have an extra WheelOfFortune.processSpinOutCome(...) method. Why not use the processor directly thus saving this method?

  3. The second if (choice == Choice.BUY_VOWEL) is not necessary since there are just 2 Choices. So, if it's not the one it's the other. Though I'd add a comment to the then bare else for clarity: // BUY_VOWEL

  4. There are 2 typos in Host.unconverConsonent(...)uncoverConsonant

  5. Why does Wheel.spin(...) depend on player and round? A wheel by itself just spins and shows (returns) a result, regardless of external entities.

  6. 500 inside Wheel.spin(...) is a magic number. It'd better be a properly named constant.

  7. I'd move the enum Choice to the top of the class. At least before the first usage in start().

  8. With start() being private one can instantiate your WheelOfFortune but cannot start it.

  9. You have a SpinOutComeProcessor but just a static processOvalBuy(...) method. I'm thinking of a BuyVowelProcessor with:

    char process(Account account) { /* calculate vowel */; return vowel; }
    

    Invoked with:

    buyVowelProcessor.process(player.getAccount());
    

    On the other hand: Doesn't a player buy a vowel from the host?

  10. According to your diagram Player is far away from, with no direct connection to your SpinOutComes. Why is it a member of them then? I'd expect a SpinOutComeProcessor to glue everything together – according to your diagram: user(?), amount, host(?), spin result.

  11. In which way does your SpinOutComeProcessor use the User and the Host?

  12. Why do you use a Character in Player.buyOval() instead of a simple char?

  13. Re ArrayList<Player> players = ... It's common practice to use the interface type in such declarations: List<Player> players = ...

  14. Why do you use an Integer for the player's amount instead of a simple int? (AFAIK there are only integer amounts so that no floating point type is needed in this case. Contrary to what is used for amounts normally.)

  15. I'd rename the Dollar... class to the more general Cash.... Such you can sell your application worldwide easily. ;-)

  16. I'd move all static members to the very top of class (before member declarations), since they belong solely to the class. All others belong to an object created from the class.

I admit, it wasn't only "at first sight" but second or third, or so. :-)

\$\endgroup\$
  • \$\begingroup\$ 1) Yes I agree. 2) This is more of readability perspective, having all the the code at the same abstraction level. 3) Yes but it is going to have more cases 4) Yes. Please ignore them for now. \$\endgroup\$ – FourOfAKind Mar 22 '15 at 5:11
  • \$\begingroup\$ 5) Yes this is one of my major concerns. I need them to create SpinOutCome. That way SpinOutCome can be tied to a player and a round. So that the processor can act on it, and it has complete information. \$\endgroup\$ – FourOfAKind Mar 22 '15 at 5:17
  • \$\begingroup\$ 11) Please refer to DollarAmoutSpinOutcomeProcessor \$\endgroup\$ – FourOfAKind Mar 22 '15 at 5:22
  • 1
    \$\begingroup\$ 3) In that case I'd use a switch statement instead of a if ... else if ... chain. That's clearer. 11) I see. \$\endgroup\$ – G.Broser says Reinstate Monica Mar 22 '15 at 5:37
  • \$\begingroup\$ 5) Is there a better way of doing it? \$\endgroup\$ – FourOfAKind Mar 22 '15 at 15:30
1
\$\begingroup\$

I prepared a few classes taking yours as template. I added one for an important entity that's missing in your diagram and in your code: Board. The idea is that:

  • methods are like messages (or commands) to an entity to do something (with something)
  • a class has methods just for the actions within its immediate area of influence
  • a class holds just references to entities it interacts with directly

Account

public class Account {

    private int amount;

    public Account(int amount) {
        this.amount = amount;
    }

    public int credit(int amount) {
        return this.amount += amount;
    }

    public int debit(int amount) {
        return this.amount -= amount;
    }

    public int amount() {
        return amount;
    }

    @Override
    public String toString() {
        return String.format("[amount=%d]", amount);
    }

} // Account

Board

public class Board extends ArrayList<Character> {

    private String sentence;
    private boolean[] visibility; // the sentence characters' visibility

    public Board(CharSequence sentence) {
        initializeWith(sentence);
    } // Board(...)

    public Board initializeWith(CharSequence sentence) {
        this.sentence = sentence.toString().toUpperCase();
        visibility = new boolean[sentence.length()];
        char c;
        for (int i = 0; i < visibility.length; i++) {
            visibility[i] = false;
            c = Character.toUpperCase(sentence.charAt(i));
            add(Character.isLetter(c) ? ' ' : c);
        }
        return this;
    }

    public boolean reveal(char character) {
        Character c = Character.toUpperCase(character);
        boolean sentenceContainsChar = false;
        for (int i = 0; i < sentence.length(); i++) {
            if (sentence.charAt(i) == c) {
                sentenceContainsChar = true;
                visibility[i] = true;
                set(i, c);
            }
        }
        return sentenceContainsChar;
    }

    public String sentence() {
        return sentence;
    }

} // Board

Host

public class Host {

    private Board board;

    public Host(Board board) {
        this.board = board;
    }

    public void doWhatsNecessaryWith(Player player, SpinResult spinResult) {
        // TODO process spin result here ...
        switch (spinResult.type()) {
        case CASH:
            player.account().credit(spinResult.amount());
            break;
        case LOSE_A_TURN:
            // TODO
            break;
        case BANKRUPT:
            // TODO
            break;
        case SOLVE_PUZZLE:
            // TODO
            break;
        }

        // ... or externalize it to a SpinResultProcessor and supply the Host with it in the constructor
        // processor.process(Player player, SpinResult spinResult);
    }

    public boolean uncoverConsonant(char consonant) {
        boolean isConsonant = false;
        // TODO check if really consonant
        isConsonant = true;
        // ------------------------------
        if (isConsonant)
            return board.reveal(consonant);
        else
            return false;
    }

    public boolean solveWith(CharSequence solution) {
        return solution.equals(board.sentence());
    }

    public void giveVowel() {
        // TODO calculate vowel
        board.reveal('e');
    }

} // Host

Player

public class Player {

    public enum Choice {
        SPIN_WHEEL, BUY_VOWEL
    }

    private String name;
    private Account account;
    private Host host;

    public Player(String name, Account account, Host host) {
        this.name = name;
        this.account = account;
        this.host = host;
    }

    public Account account() {
        return account;
    }

    public void buyVowel() {
        host.giveVowel();
    }

    // Choice makeChoice() {
    // return null;
    // }

    public boolean guessConsonant(char consonant) {
        return host.uncoverConsonant(consonant);
    }

    public SpinResult spin(Wheel wheel) {
        return wheel.spin();
    }

    public void giveToHost(SpinResult spinResult) {
        host.doWhatsNecessaryWith(this, spinResult);
    }

    public boolean solveWith(CharSequence solution) {
        return host.solveWith(solution);
    }

    public void acceptCashGift(int amount) {
        account.debit(amount);
    }

    public String name() {
        return name;
    }

    @Override
    public String toString() {
        return String.format("[name=%s, account=%s]", name, account);
    }

} // Player

Wheel

Though your SpinOutCome is really object-oriented I used a SpinResult with an amount that's supposed to be deliberately ignored if its type isn't CASH. Thus saving a few classes.

public class Wheel {

    public enum SpinResultType {
        CASH, LOSE_A_TURN, BANKRUPT, SOLVE_PUZZLE
    }

    public SpinResult spin() {
        // TODO calculate random spin result
        return new SpinResult(SpinResultType.CASH, 100);
    }

    /**
     */
    public class SpinResult {
        private SpinResultType type;
        private int amount;

        public SpinResult(SpinResultType type) {
            this(type, 0);
        }

        public SpinResult(SpinResultType type, int amount) {
            this.type = type;
            this.amount = amount;
        }

        public SpinResultType type() {
            return type;
        }

        public int amount() {
            return amount;
        }

        @Override
        public String toString() {
            return String.format("[type=%s, amount=%s]", type, amount);
        }

    } // SpinResult

} // Wheel

GamePlay

public class GamePlay {

    public static void main(String[] args) {

        String sentence = "The answer is forty-two!";
        Board board = new Board(sentence);
        Wheel wheel = new Wheel();
        Host host = new Host(board);

        List<Player> players = new ArrayList<Player>();
        players.add(new Player("FourOfAKind", new Account(0), host));
        players.add(new Player("Gerold Broser", new Account(0), host));

        for (Player player : players) {
            System.out.printf("Player: %s%n", player.name());

            String niceTry = "nice try";
            System.out.printf("Solved with '%s'? %b%n", niceTry, player.solveWith(niceTry));

            SpinResult sr = player.spin(wheel);
            System.out.printf("SpinResult %s%n", sr);
            player.giveToHost(sr);
            System.out.println(player);

            System.out.printf("x good guess? %-5b %s%n", player.guessConsonant('x'), board);
            System.out.printf("t good guess? %-5b %s%n", player.guessConsonant('t'), board);
            player.buyVowel();
            System.out.printf("Bought vowel: %s%n", board);

            System.out.println();
        }
    } // main(...)

} // GamePlay

Prints:

Player: FourOfAKind
Solved with 'nice try'? false
SpinResult [type=CASH, amount=100]
[name=FourOfAKind, account=[amount=100]]
x good guess? false [ ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  , -,  ,  ,  , !]
t good guess? true  [T,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  ,  , T,  , -, T,  ,  , !]
Bought vowel: [T,  , E,  ,  ,  ,  ,  , E,  ,  ,  ,  ,  ,  ,  ,  , T,  , -, T,  ,  , !] 

Player: Gerold Broser
Solved with 'nice try'? false
SpinResult [type=CASH, amount=100]
[name=Gerold Broser, account=[amount=100]]
x good guess? false [T,  , E,  ,  ,  ,  ,  , E,  ,  ,  ,  ,  ,  ,  ,  , T,  , -, T,  ,  , !]
t good guess? true  [T,  , E,  ,  ,  ,  ,  , E,  ,  ,  ,  ,  ,  ,  ,  , T,  , -, T,  ,  , !]
Bought vowel: [T,  , E,  ,  ,  ,  ,  , E,  ,  ,  ,  ,  ,  ,  ,  , T,  , -, T,  ,  , !] 

I also added a few simple test classes. For proper testing I'd use a testing framework like JUnit. As things progress I'd also use a build management tool like Apache Maven.

AccountTest

public class AccountTest {

    public static void main(String[] args) {

        Account a = new Account(0);
        System.out.println(a.credit(100)); // 100
        System.out.println(a.debit(50)); // --> 50
        System.out.println(a.debit(100)); // --> -50
        System.out.println(a.amount()); // -50
        System.out.println(a);
    }

} // AccountTest

BoardTest

public class BoardTest {

    public static void main(String[] args) {

        String sentence = "The answer is forty-two!";
        Board board = new Board(sentence);
        System.out.printf("Sentence %8s%n", board.sentence());
        System.out.printf("Board    %8s%n", board);

        System.out.printf("t? %-5b %s%n", board.reveal('t'), board);
        System.out.printf("x? %-5b %s%n", board.reveal('x'), board);
        System.out.printf("e? %-5b %s%n", board.reveal('e'), board);
        System.out.printf("u? %-5b %s%n", board.reveal('u'), board);
    }

} // BoardTest

HostTest

public class HostTest {

    public static void main(String[] args) {

        String sentence = "The answer is forty-two!";
        Board board = new Board(sentence);
        Host host = new Host(board);

        String notTheSolution = "This is not the solution";
        System.out.printf("Is '%s' the solution of '%s'? %b%n", notTheSolution, board.solution(), host.solveWith(notTheSolution));
        System.out.printf("Is '%s' the solution of '%s'? %b%n", sentence, board.solution(), host.solveWith(sentence));

    }

} // HostTest
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.