Slot Machine Simulator

Question

I'm studying alone, so I don't know if I'm writing efficiently. Could this slot machine simulator be improved? How?

import acm.program.*;
import acm.util.*;

/*
 * THIS PROGRAM SIMULATES THE SLOT MACHINE
 */

public class cap6ex5 extends ConsoleProgram{

private RandomGenerator rgen = RandomGenerator.getInstance();

private int money = 50; //starting stake

private String value = "";



public void run(){


    String instAnswer = readLine("Would you like instructions?");

    while (!instAnswer.equals("yes") && !instAnswer.equals("no")) {
        instAnswer = readLine("Answer yes or no.");
    }
    if (instAnswer.equals("yes")) println("YOU SHOUD KNOW!");


    startGame(money);

}
private void startGame(int x){
    String keepPlaying = "";
    int finalMoney = x;
    int prize = 0;
    while (finalMoney > 0){
        keepPlaying = readLine("You have $" + finalMoney +". Would you like to play?");
        //avoid different answers
        while(true){ 
            if (keepPlaying.equals("yes") || keepPlaying.equals("no")) break;
            keepPlaying = readLine("Asnwer yes or no.");

        }
        if (keepPlaying.equals("no")) break;

        finalMoney--;
        prize = gamePrize();
        finalMoney += prize;
        println(value + "     -- You win $" + prize);
    }

    println("Okay, bye. You ended with $" + finalMoney);
}
private int gamePrize(){
    int countBAR = 0;   //BAR == 1
    int countBELL = 0;  //BELL == 2
    int countPLUM = 0;  //PLUM == 3
    int countORANGE = 0;//ORANGE == 4
    int countCHERRY = 0;//CHERRY == 5
    value = "";


    for (int i = 1; i <= 3 ; i++){
        int x = rgen.nextInt(1, 6);

        switch(x){
            case 1:
                countBAR++;
                value += "BAR ";
                break;
            case 2:
                countBELL++;
                value += "BELL ";
                break;
            case 3:
                countPLUM++;
                value += "PLUM ";
                break;
            case 4:
                countORANGE++;
                value += "ORANGE ";
                break;
            case 5:
                countCHERRY++;
                value += "CHERRY ";
                break;
            case 6: value += "LEMON "; break;
        }

    }

    return result(countBAR, countBELL, countPLUM, countORANGE, countCHERRY);

}
private int result(int bar, int bell, int plum, int orange, int cherry){
    int prize = 0;
    if (bar == 3){
        prize = 250;
    } else if ((bell > 1 && bar ==1) || bell == 3){
        prize = 20;
    } else if ((plum > 1 && bar == 1) || plum == 3){
        prize = 14;
    } else if ((orange >1 && bar == 1) || orange == 3){
        prize = 10;
    } else switch(cherry){
        case 1: prize = 7; break;
        case 2: prize = 5; break;
        case 3: prize = 2; break;
    }
    return prize;
}



}

Here are the prize rules:

BAR BAR BAR = $250
BELL BELL BELLorBAR = $20
PLUM PLUM PLUMorBAR = $14
ORANGE ORANGE ORANGEorBAR = $10
CHERRY CHERRY CHERRY = $7
CHERRY CHERRY --- = $5
CHERRY --- --- = $2

Answer 1

Casinos are a shady business, but I'm even more suspicious of a slot machine that offers me a random prize without displaying the result of the spin.

Infrastructure

I don't recommend developing a habit of relying on non-standard libraries that don't do very much. For example, it looks like you are using a ConsoleProgram class that provides a run() method for you to fill in, as well as inherited methods println() and readLine(). Idiomatic Java would just have a public static void main(String[] args) function, System.out.println(), and Scanner.nextLine(). You are also using a RandomGenerator, when you could just use the standard Random class.

If you are learning from a book that is encouraging you to use those calls, then I suggest that you find a different book that teaches you more transferrable skills.

Object-oriented programming

Your class has a public run() method; everything else is a black box. That makes the code hard to reuse and impossible to unit-test.

How should the code be organized? I recommend defining a class to model the slot machine with the following public interface:

public class SlotMachine {
    public enum Symbol {
        BAR, BELL, PLUM, ORANGE, CHERRY, LEMON;
    }

    public SlotMachine() {
        …
    }

    /**
     * Spins all wheels and returns the payout.
     */
    public int spin() {
        …
    }

    /**
     * Returns the symbol on wheel 0, 1, or 2.
     */
    public Symbol wheel(int wheel) {
        …
    }

    /**
     * Returns a space-separated string of the symbols on the wheels.
     */
    public String toString() {
        …
    }
}

In a sense, my public spin() method is equivalent to your private gamePrize() method, but with a name that suggests that an action is taking place.

Here is how I would fill in that outline. Note that I've used some more advanced language features: an enum for the symbols and an EnumMap to count the symbols. Your countBAR, countBELL, etc. may be more suitable for a beginner.

import java.util.EnumMap;
import java.util.Random;

public class SlotMachine {
    public enum Symbol {
        BAR, BELL, PLUM, ORANGE, CHERRY, LEMON;
    }

    private Random rgen = new Random();
    private Symbol[] wheels = new Symbol[3];

    private static final EnumMap<Symbol, Integer> ZERO_COUNT;
    static {
        ZERO_COUNT = new EnumMap<>(Symbol.class);
        for (int s = 0; s < Symbol.values().length; s++) {
            ZERO_COUNT.put(Symbol.values()[s], 0);
        }
    }

    /**
     * Spins all wheels and returns the payout.
     */
    public int spin() {
        EnumMap<Symbol, Integer> symCounts = new EnumMap<>(ZERO_COUNT);
        for (int i = 0; i < wheels.length; i++) {
            // http://stackoverflow.com/q/1972392
            int randInt = rgen.nextInt(Symbol.values().length);
            this.wheels[i] = Symbol.values()[randInt];
            symCounts.put(this.wheels[i], 1 + symCounts.get(this.wheels[i]));
        }
        return payout(symCounts);
    }

    private int payout(EnumMap<Symbol, Integer> ct) {
        if (ct.get(Symbol.BAR) == 3) {
            return 250;
        } else if (ct.get(Symbol.BELL) == 3 ||
                   ct.get(Symbol.BELL) == 2 && ct.get(Symbol.BAR) == 1) {
            return 20;
        } else if (ct.get(Symbol.PLUM) == 3 ||
                   ct.get(Symbol.PLUM) == 2 && ct.get(Symbol.BAR) == 1) {
            return 14;
        } else if (ct.get(Symbol.ORANGE) == 3 ||
                   ct.get(Symbol.ORANGE) == 2 && ct.get(Symbol.BAR) == 1) {
            return 10;
        } else switch (ct.get(Symbol.CHERRY)) {
            case 3:  return 7;
            case 2:  return 5;
            case 1:  return 2;
            default: return 0;
        }
    }

    /**
     * Returns the symbol on wheel 0, 1, or 2.
     */
    public Symbol wheel(int wheel) {
        if (wheel < 0 || wheel >= 3) {
            throw new IllegalArgumentException();
        }
        return this.wheels[wheel];
    }

    /**
     * Returns a space-separated string of the symbols on the wheels.
     */
    public String toString() {
        return this.wheel(0) + " " + this.wheel(1) + " " + this.wheel(2);
    }
}

With the SlotMachine class defined, you can write more expressive code in your main() function, like:

SlotMachine sm = new SlotMachine();
int payout = sm.spin();
System.out.printf("Spin result = %s.  You win $%d%n", sm, payout);

Answer 2

• When comparing against "yes" or "no", you should consider using equalsIgnoreCase so that answers like YES can be accepted. (Perhaps the player is excited to play?)
• startGame: I cannot tell what the x argument is. It should be renamed.
• gamePrize: Rather than hard-coding the 1-6 figures in your switch statement, you should make each one a constant (ex. VAL_ORANGE).
• result: I would not know that you arguments are the number of occurrences of each type. These should probably be renamed.
• result: You currently have 1 cherry as winning $7 and 3 as winning $2. This appears to be a bug.

Answer 3

Naming

• You shouldn't shorten variable names, as it makes code less readable. Without the actual displayed text, I would for example not know what an inst is.
• value is a very generic name, especially for a field.
• I get the idea behind the various count names, but I would still prefer if the names followed standard patterns. So countBAR would be barCount, etc.
• You switch your variable names from countBAR to bar in the result function. But it's still a count, so it should be named the same to avoid confusion.

Comments

Inline comments should explain why the code is written the way it is written. These comments however do not clarify anything, they are just confusing:

int countBAR = 0;   //BAR == 1
int countBELL = 0;  //BELL == 2
int countPLUM = 0;  //PLUM == 3
int countORANGE = 0;//ORANGE == 4
int countCHERRY = 0;//CHERRY == 5

Structure

The above comment problem seems to stem from a lack of structure.

Currently, you present Bar, Bell, etc as string as well as integers, which causes some confusion.

You should restructure your code so that these values are encapsulated in the same structure, as they belong together. You could create an object for this, or you could simply use an enum containing Bar, Bell, etc. There isn't really a need to associate an integer with it, you can simply retrieve a random value from the enum.