Fruit machine game

Question

I'm a beginner in Java, and this is the first time I have written a program from scratch without a guide explaining how and where to create classes etc. Please review my code to see whether the program has been written in a logical way.

The program is a very basic FruitMachine. The user clicks on "spin", and three cards are drawn. Depending on what these 3 cards are the user either wins or loses points. The game continues until a certain threshold is met.

FruitMachine Class - Main class, creates a UserInterFaceFrame

    public class FruitMachine {

    public static void main(String[] args) {
        UserInterfaceFrame userInterface = new UserInterfaceFrame("Fruitmachine");
    }

}

GameLogic class - Controls the functioning of the game

import java.beans.PropertyChangeSupport;
import java.util.Random;

public class GameLogic {

private String[] cards;
private String[] drawnCards;
private int balance;


public GameLogic() {
    cards = new String[]{"Ace", "King", "Queen", "Jack", "Joker"};
    drawnCards = new String[3];
    this.balance = 100;
}

public String[] getCards() {
    return cards;
}

public int getBalance() {
    return balance;
}

public String getDrawnCardAtX(int x) {
    return drawnCards[x];
}

public void setBalance(int newBalance) {
    balance = newBalance;
}

public String spin() {

    Random random = new Random();
    int count = 0;
    for (int i = 0; i < 3; i++) {

        drawnCards[i] = cards[random.nextInt(cards.length)];
        System.out.println(drawnCards[i]);

        if (drawnCards[i].equals("Joker")) {
            balance -= 25;
            count++;
        }
    }
    if (count > 1) {
        return count + " jokers: you lose " + (25 * count) + " points";
    }
    if (count != 0) {
        return count + " joker: you lose " + (25 * count) + " points";
    }

    if (drawnCards[0].equals(drawnCards[1]) && drawnCards[1].equals(drawnCards[2])) {
        balance += 50;
        return "Three of a kind - you win 50 points";

    } else if (drawnCards[0].equals(drawnCards[1]) || drawnCards[0].equals(drawnCards[2])
            || drawnCards[1].equals(drawnCards[2])) {

        balance += 20;
        return "Two of a kind - you win 20 points";
    }
    return null;
}
}

UserInterFace class - Creates a JFrame and fills it with JPanels for the user interface.

import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import javax.swing.BorderFactory;
import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.SwingConstants;

public class UserInterfaceFrame extends JFrame implements ActionListener {
private GameLogic gamelogic;

;

public UserInterfaceFrame(String title) {
    super(title);
    setSize(600, 400);
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    setVisible(true);
    setLayout(new GridLayout(2, 0));

    this.gamelogic = new GameLogic();

    JPanel labelsBorder = new JPanel();
    labelsBorder.setBorder(BorderFactory.createLoweredBevelBorder());
    JPanel labels = new JPanel();
    labels.setLayout(new BoxLayout(labels, BoxLayout.Y_AXIS));

    DataLabel balanceLabel = new DataLabel("Balance is " + gamelogic.getBalance());
    labels.add(balanceLabel);

    DataLabel cards = new DataLabel("Welcome");
    labels.add(cards);

    DataLabel winLose = new DataLabel("");
    labels.add(winLose);

    labelsBorder.add(labels);

    JPanel lowerFrame = new JPanel();
    lowerFrame.setLayout(new GridLayout(0, 2));

    JPanel lowerRightFrame = new JPanel();
    lowerRightFrame.setLayout(new GridLayout(2, 0));
    lowerRightFrame.setBorder(BorderFactory.createEmptyBorder(60, 60, 60, 60));

    JPanel lowerLeftFrame = new JPanel();
    lowerLeftFrame.setLayout(new GridLayout(0, 3, 30, 0));
    lowerLeftFrame.setBorder(BorderFactory.createEmptyBorder(50, 50, 50, 50));
    JPanel card1 = new CardPanel();
    JLabel card1Label = new JLabel("", SwingConstants.CENTER);
    card1.add(card1Label);

    JPanel card2 = new CardPanel();
    JLabel card2Label = new JLabel("", SwingConstants.CENTER);
    card2.add(card2Label);

    JPanel card3 = new CardPanel();
    JLabel card3Label = new JLabel("", SwingConstants.CENTER);
    card3.add(card3Label);

    lowerLeftFrame.add(card1);
    lowerLeftFrame.add(card2);
    lowerLeftFrame.add(card3);

    JButton spin = new JButton("Spin");
    spin.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent e) {
            cards.setText(gamelogic.spin());
            balanceLabel.setText("Balance is " + gamelogic.getBalance());
            card1Label.setText(gamelogic.getDrawnCardAtX(0));
            card2Label.setText(gamelogic.getDrawnCardAtX(1));
            card3Label.setText(gamelogic.getDrawnCardAtX(2));

        }
    });

    JButton reset = new JButton("New game");
    reset.setEnabled(false);
    reset.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            gamelogic.setBalance(100);
            reset.setEnabled(false);
            spin.setEnabled(true);
            winLose.setText("");
            balanceLabel.setText("Balance is: " + gamelogic.getBalance());

        }
    });

    balanceLabel.addPropertyChangeListener(new PropertyChangeListener() {
        @Override
        public void propertyChange(PropertyChangeEvent event) {
            if (gamelogic.getBalance() < 0) {
                winLose.setText("You lose!");
                reset.setEnabled(true);
                spin.setEnabled(false);
            }
            if (gamelogic.getBalance() > 150) {
                winLose.setText("You win!");
                reset.setEnabled(true);
                spin.setEnabled(false);
            }
        }
    });

    lowerRightFrame.add(spin);
    lowerRightFrame.add(reset);

    lowerFrame.add(lowerLeftFrame);
    lowerFrame.add(lowerRightFrame);

    add(labelsBorder);
    add(lowerFrame);

    setVisible(true);
}

@Override
public void actionPerformed(ActionEvent e) {
    // TODO Auto-generated method stub

}

}

DataLabel Class - Extends JPanel to overwrite some of the default behaviour

    import javax.swing.BorderFactory;
import javax.swing.JLabel;

public class DataLabel extends JLabel {

    public DataLabel(String setString) {
        super(setString);
        setBorder(BorderFactory.createEmptyBorder(20, 20, 20, 20));
    }

}

Card Panel Class

import java.awt.Color;
import java.awt.GridBagLayout;
import javax.swing.BorderFactory;
import javax.swing.JPanel;

public class CardPanel extends JPanel {

    public CardPanel() {
        setBorder(BorderFactory.createRaisedBevelBorder()); // Creates a raised border around the edge of the cards to make them "pop".
        setBackground(Color.yellow); // Sets the colour of the cards to yellow
        setLayout(new GridBagLayout()); // GridBagLayout with out any constraints will always centre a JLabel.
    }
}

Answer 1

'Card' could be an ENUM. This should also be a static final class variable, for example:

private static final Card[] CARDS = new Card[]{Card.ACE, Card.KING, Card.QUEEN, Card.JACK, Card.JOKER};

Avoid magic numbers, the '3' here should be declared, same for the starting balance

private static final int NUMBER_OF_CARDS_DRAWN = 3;
private static final int STARTING_BALANCE = 100;

Here you use the magic number '3' again, which should be changed to variable

// Changed '3' to 'NUMBER_OF_CARDS_DRAWN'
drawnCards = new String[NUMBER_OF_CARDS_DRAWN];
...
for (int i = 0; i < NUMBER_OF_CARDS_DRAWN; i++) {

This does not make sense. Count is never decremented so the second if statement will never be reached. You can remove it.

if (count > 1) {
    return count + " jokers: you lose " + (25 * count) + " points";
}
// You can remove this.
if (count != 0) {
    return count + " joker: you lose " + (25 * count) + " points";
}

Remove magic numbers '25, '50', '20'. Use static variables

Avoid having game logic inside your UI.

The number of cards should be retrieved from the GameLogic, then add that number of cards.

// Needs to be refactored into a for-loop, given the NumberOfCards is a variable.
lowerLeftFrame.add(card1);
lowerLeftFrame.add(card2);
lowerLeftFrame.add(card3);

Same here, if you have an ArrayList of labels, you'll need to refactor this into a for-loop.

// Needs to be refactored into a for-loop, given the NumberOfCards is a variable.
card1Label.setText(gamelogic.getDrawnCardAtX(0));
card2Label.setText(gamelogic.getDrawnCardAtX(1));
card3Label.setText(gamelogic.getDrawnCardAtX(2));

This should not be in the UI. It's definetly GameLogic. Consider moving to constructor of GameLogic

// Doesn't belong in UI. Consider putting it in the constructor of GameLogic
gamelogic.setBalance(100);
reset.setEnabled(false);
spin.setEnabled(true);

This is very much GameLogic and does not belong in the UI

// This is very much GameLogic, doesn't belong in UI
if (gamelogic.getBalance() < 0) {
    winLose.setText("You lose!");
    reset.setEnabled(true);
    spin.setEnabled(false);
}
if (gamelogic.getBalance() > 150) {
    winLose.setText("You win!");
    reset.setEnabled(true);
    spin.setEnabled(false);
}

Answer 2

Let's show how to split the model up and remove all the UI elements.

First of all, we want to play a card game, so let's define the cards:

public enum Card {
    ACE, KING, QUEEN, JACK, JOKER;
}

---

We have some kind of account with points:

import java.util.function.Predicate;

public final class Account {
    private int balance;

    public Account(int startingBalance) {
        this.balance = startingBalance;
    }

    public int getBalance() {
        return balance;
    }

    public void withdraw(int loss) {
        balance -= loss;
    }

    public void deposit(int gain) {
        balance += gain;
    }

    public boolean test(Predicate<Integer> test) {
        return test.test(balance);
    }

    @Override
    public String toString() {
        return String.format("Account has %d points", balance);
    }
}

So that god rid of that part of the equations, we don't want anything in the machine description itself if we can separate it out. Note that we don't supply a "setter".

---

Now we have multiple things resulting from a single spin. Somehow we must present them in the GUI, but without directly calling the GUI. Tricky? Not that tricky, just use a listener interface that the GUI classes can implement:

import java.util.List;

public interface SpinListener {
    void showCardSpin(List<Card> cards);
    void showResult(String result);
}

---

Fine, now let's implement the game:

import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Random;

public final class FruityMacFruitMachine {
    // in the end just a list of all the cards as an unmodifiable constant value
    private final static List<Card> CARDS =
            Collections.unmodifiableList(new ArrayList<Card>(EnumSet.allOf(Card.class)));
    private static final int CARD_COUNT = 3;

    private static final int LOSS_PER_JOKER = 25;
    private static final int WINNINGS_FOR_THREE_OF_A_KIND = 50;
    private static final int WINNINGS_FOR_TWO_OF_A_KIND = 20;

    // I'm a security buff, secure randoms are *more* random than Random
    private final Random random = new SecureRandom();
    private final Account account;
    private SpinListener listener;

    // we create the machine with an account and a listener for the results
    public FruityMacFruitMachine(Account account, SpinListener listener) {
        this.account = account;
        this.listener = listener;
    }

    // create a card spin, clearly a separate method
    private List<Card> cardSpin(int amountOfCards) {
        List<Card> drawnCards = new ArrayList<Card>(amountOfCards);
        for (int i = 0; i < amountOfCards; i++) {
            drawnCards.add(CARDS.get(random.nextInt(CARDS.size())));
        }
        return drawnCards;
    }

    // the spin and all the followup moves
    public void spin() {
        // take a spin
        List<Card> draw = cardSpin(CARD_COUNT);
        // the GUI should not change the draw, right?
        listener.showCardSpin(Collections.unmodifiableList(draw));

        // check for jokers by iterating over the cards,
        // filtering out the jokers and then counting them
        int jokers = (int) draw.stream().filter(card -> card == Card.JOKER).count();
        if (jokers >= 1) {
            // we reuse the loss, no need to calculate twice!
            int loss = LOSS_PER_JOKER * jokers;
            account.withdraw(loss);
            listener.showResult(String.format("%d joker(s): you lose %d points", jokers, loss));
            return;
        }

        // we now count the distinct cards
        int distinctCards = (int) draw.stream().distinct().count();

        // all cards are distinct -> no luck
        if (distinctCards == CARD_COUNT) {
            listener.showResult("No luck, draw again");
            return;
        }

        // we just declare the variables in advance
        // and then initialize them in the if / else code blocks
        int gain;
        String result;
        if (distinctCards == 1) {
            gain = WINNINGS_FOR_THREE_OF_A_KIND;
            result = "Three of a kind";
        } else if (distinctCards == 2) {
            gain = WINNINGS_FOR_TWO_OF_A_KIND;
            result = "Two of a kind";
        } else {
            throw new IllegalStateException();
        }
        account.deposit(gain);
        listener.showResult(String.format("%s: you win %d points", result, gain));
    }
}

---

Now here is a console implementation of the game:

public static void main(String[] args) {
    Account account = new Account(100);
    FruityMacFruitMachine game = new FruityMacFruitMachine(account, new SpinListener() {
        @Override
        public void showCardSpin(List<Card> cards) {
            System.out.println(cards);
        }

        @Override
        public void showResult(String result) {
            System.out.println(result);
        }
    });
    while (account.test(x -> x >= 50 && x < 200)) {
        game.spin();
        System.out.println(account);
        System.out.println();
    }
    System.out.println("Thank you for playing, come again");
}

Up to you go create a GUI around it by implementing the listener and some model / view for the Account.