HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

Basic OOP Poker - Deck, Cards and Hands

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pokercardsdeckhandsandoopbasic

Problem

I decided it would be interesting to simulate a game of Poker. Baby steps at the moment, and eventually I'll attempt to turn it into a GUI. The code I have so far is very basic such as populating a deck, shuffling the deck, and distributing two cards to a player. Trying to understand how inheritance works, the proper use of Java generics, and grasping the concept of how powerful/fun OOP can be. The Collections Framework is also something I'm beginning to experiment with.

I would really appreciate anything that will help me better understand Java.

import java.util.*;

class Deck {
    // create possible card combinations
    public final String[] SUITS = { "H", "D", "C", "S" };
    public final String[] RANKS = { "A", "K", "Q", "J", "10", "9", "8", "7", "6", "5", "4", "3", "2" };

    // maximum number of cards
    public final int deckLength = SUITS.length * RANKS.length;
    public List fullDeck = new ArrayList<>();

    public Deck() {
        for(int i = 0; i  fullDeck) {
        this.fullDeck = fullDeck;
        for(int i = 0; i  shuffle(List fullDeck) {
        this.fullDeck = fullDeck;
        Collections.shuffle(fullDeck);
        return fullDeck;
    }

    // this was mainly used for testing purposes
    // to ensure shuffling was indeed taking place
    public void showDeck(List fullDeck) {
        this.fullDeck = fullDeck;
        for(int i = 0; i  fullDeck) {
        super.fullDeck = fullDeck;
        for(int i = 0; i  cards = new ArrayList<>();
        Deck deck = new Deck(cards);
        deck.shuffle(cards);

        Hands hands = new Hands();
        hands.getHand(cards);
        hands.showHand();

    }
}

Solution

Welcome to Code Review, and welcome to Java! Overall your code is clean and easy to understand, so thank you for that.

Enums

Storing these values in a string array is not very extensible:

public final String[] SUITS = { "H", "D", "C", "S" };
public final String[] RANKS = { "A", "K", "Q", "J", "10", "9", "8", "7", "6", "5", "4", "3", "2" }


The first thing to note is that a List provides more options and flexibility than a raw array. However, I think it would be best to store these values as an Enum:

public enum SUITS {
    H, D, C, S
}


This will allow you to use these values in a switch statement, and provides other benefits. Most of all, it is more readable and more extensible.

Variable Names

I would strongly recommend against single letter variable names outside of simple constructs such as for (int i = 0; i < something; i++);. Instead of H, D, C, and S, I would just spell them out as HEARTS, DIAMONDS, etc. There is no disadvantage to using the extra characters to make the code more readable. In the rest of your code you do not make this mistake, so that is good.

Improper OOP

I do not think that Hand should inherit from Deck:

class Hands extends Deck {


I would advise that you think about inheritance as an "is a" relationship. A hand is not a deck, and should not be treated like one. For example, it is unlikely that you will need to "shuffle" a hand. To me, this is the same as having Hand or Deck inherit from a Card. It is not going to end up being helpful to the organization or understanding of the code.

I don't understand why Cards is a class:

public class Cards {


A singular Card should be the class here. Then, the Hand and Deck can have a "has a" relationship with cards, and use Card objects. The way you have this set up here:

List cards = new ArrayList<>();
    Deck deck = new Deck(cards);
    deck.shuffle(cards);

    Hands hands = new Hands();
    hands.getHand(cards);
    hands.showHand();


really does not make a whole lot of sense. Why would a Cards object have Decks or Hands as objects? It should definitely be the other way around.

I look forward to seeing a revised version of your code in another question!

Code Snippets

public final String[] SUITS = { "H", "D", "C", "S" };
public final String[] RANKS = { "A", "K", "Q", "J", "10", "9", "8", "7", "6", "5", "4", "3", "2" }
public enum SUITS {
    H, D, C, S
}
class Hands extends Deck {
public class Cards {
List<String> cards = new ArrayList<>();
    Deck deck = new Deck(cards);
    deck.shuffle(cards);

    Hands hands = new Hands();
    hands.getHand(cards);
    hands.showHand();

Context

StackExchange Code Review Q#65134, answer score: 24

Revisions (0)

No revisions yet.