OOP Design of a Generic Deck of Cards

Question

I have structured a package containing interfaces that could be implemented and used for any card game.

I'm interested in knowing if I'm on the right track OOP-wise, or If I need to change my design.

ICard.java

package com.tn.deck;


public interface ICard {
    void printCard();
}

IDeck.java

package com.tn.deck;


public interface IDeck {
    ICard dealCard();
    void shuffle();
}

IHand.java

package com.tn.deck;


public interface IHand {
    void addCard(ICard card);
    void removeCard(ICard card);
    void showHand();
    int calculateValue();
}

IRank.java

package com.tn.deck;


public interface IRank {
    int[] addRanks();
}

ISuit.java

package com.tn.deck;


public interface ISuit {
    String[] addSuits();
}

Here is an example usage of the interfaces when I want to create a game of BlackJack:

Card.java

package com.tn.blackjack;

import com.tn.deck.ICard;


public class Card implements ICard {
    private String suit;
    private int rank;

    Card(String suit, int rank) {
        this.suit = suit;
        this.rank = rank;
    }

    public String getSuit() {
        return suit;
    }

    public int getRank() {
        return rank;
    }

    @Override
    public void printCard() {
        System.out.printf("%s%d ", suit, rank);
    }
}

Deck.java

package com.tn.blackjack;

import com.tn.deck.ICard;
import com.tn.deck.IDeck;

import java.util.ArrayList;
import java.util.List;


public class Deck implements IDeck {
    private int sizeOfDeck;
    private List<ICard> cards;

    Deck(int sizeOfDeck) {
        this.sizeOfDeck = sizeOfDeck;
        this.cards = new ArrayList<>(sizeOfDeck);
    }

    void initializeWith(String[] suits, int[] ranks) {
        for(int i = 0; i < sizeOfDeck; i++) {
            cards.add(new Card(
                    suits[i % suits.length],
                    ranks[i % ranks.length]
            ));
        }
        print();
    }

    public int getSize() {
        return cards.size();
    }

    private void print() {
        for(ICard card : cards) {
            card.printCard();
        }
    }

    @Override
    public ICard dealCard() {
        if(cards.size() < 1) {
            throw new IllegalArgumentException("The deck is empty");
        }
        int index = new Random().nextInt(cards.size());
        ICard card = cards.get(index);
        cards.remove(index);
        return card;
    }

    @Override
    public void shuffle() {
        Collections.shuffle(cards);
    }
}

Player.java

package com.tn.blackjack;

import com.tn.deck.ICard;
import com.tn.deck.IHand;

import java.util.ArrayList;
import java.util.List;


public class Player implements IHand{
    private int id;
    private int numberOfCardsAllowed;
    private List<ICard> cards;

    Player(int id, int numberOfCardsAllowed) {
        this.id = id;
        this.numberOfCardsAllowed = numberOfCardsAllowed;
        this.cards = new ArrayList<>(numberOfCardsAllowed);
    }

    public int getId() {
        return id;
    }

    @Override
    public void addCard(ICard card) {
        if(cards.size() > numberOfCardsAllowed) {
            throw new IllegalArgumentException("You are not allowed to draw more cards");
        }
        cards.add(card);
    }

    @Override
    public void removeCard(ICard card) {

    }

    @Override
    public void showHand() {
        for(ICard card : cards) {
           card.printCard();
        }
    }

    @Override
    public int calculateValue() {
        int score = 0;
        for(ICard card : cards) {
            score += ((Card)card).getRank();
        }
        return score;
    }
}

BlackJack.java

package com.tn.blackjack;

import com.tn.deck.ICard;
import com.tn.deck.IRank;
import com.tn.deck.ISuit;


public class BlackJack implements ISuit, IRank {
    private static int DECK_SIZE = 52;
    private static int MAX_NUMBER_OF_CARDS = 22;

    private String[] suits;
    private int[] ranks;
    private Deck deck;
    private Player[] players;

    public BlackJack() {
        this.suits = addSuits();
        this.ranks = addRanks();
        this.deck = new Deck(DECK_SIZE);
        this.players = new Player[] {
                new Player(1, MAX_NUMBER_OF_CARDS),
                new Player(2, MAX_NUMBER_OF_CARDS)
        };
        deck.initializeWith(suits, ranks);
    }

    public ICard dealCard() {
        return deck.dealCard();
    }

    @Override
    public String[] addSuits() {
        return new String[] {"\u2660", "\u2665", "\u2666", "\u2663"};
    }

    @Override
    public int[] addRanks() {
        return new int[] {2, 3, 4, 5, 6, 7, 8, 9, 10, 10, 10, 10, 11};
    }
}

Answer 1

Thanks for sharing the code.

Naming

Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

Naming Conventions.

On the bright side you follow the Java Naming Conventions

But you prefix you interfaces with I. Currently this is considered "old fashioned". I personally don't do it because my IDEs auto completion suggests variables starting with an i in that case, and there is no point in telling by the name of a variable that it is declared as an interface type...

In general names of interfaces should be based on adjectives where possible.

Do not surprise the reader

You have method names like addSuits() which suggests that you put something into the object but instead your methods have no parameter and return something from the object.

Also: If an object is a "Rank" what sense does it make to call iRank.addRank()?

OOP

Doing OOP means that you follow certain principles which are (amongst others):

• information hiding / encapsulation
• single responsibility
• separation of concerns
• KISS (Keep it simple (and) stupid.)
• DRY (Don't repeat yourself.)
• "Tell! Don't ask."
• Law of demeter ("Don't talk to strangers!")

Interfaces, abstract classes, or inheritance support hat principles and should be used as needed. They do not "define" OOP.

interfaces

Interfaces should be uses if you plan to create multiple classes sharing the same methods. Sometime it is usefull to create an interface for a class with complex/expensive behavior in order to decouple it, eg.: to enable UnitTesting.

In your exampe a good interfaces meight be:

interface Suitable {
    booleanIsSameSuit(Suitable other);
}
interface Rankable {
   booleanIsConsecutive(Rankable other);
}

inharitence

We use inharitance to extend or redefine the behavior of a base class (including interfaces). Inheritance represents an is a relationship between the base class and the sub class.

A bad example ist your class BlackJack: The game itself does not have "Suits" and "Ranks", therefore it should not have the methods addSuits() nor addRanks()

In your example for a concrete game (BlackJack) you could implement the cards like this:

enum Suit {SPADE, CLUB, DIAMOND, HEART};
enum Rank {R2,R3,R4,R5,R6,R7,R8,R9,R10,JACK,QUEEN,KING,ACE}

public class Card implements Suitable, Rankable, Comparable<Card>{
  private final Rank rank;
  private final Suit suit;
  public Card(Suit suit, Rank rank){
     this.suit = suit;
     this.rank = rank;
  }
  
  @Override 
  public int compareTo(Card other){
    // compare rank and suit
  }
  
  @Override 
  public booleanIsSameSuit(Suitable other){
    return suit.equals(other.suit);
  }
  
  @Override 
  public booleanIsConsecutive(Rankable other){
    return 1+rank.ordinal()==other.rank.ordinal()
        ||  other.rank.ordinal()==rank.ordinal()-1;
  } 
}

This could be used like this:

List<Card> deck = createDeck(Suit.values(),Rank.values());

// ...

private List<Card> createDeck(Suit[] suits, Rank[] ranks){
    List<Card> newDeck = new ArrayList<>();
    for(Suit suit : suits)
      for(Rank rank : ranks)
         newDeck.add(new Card(suit,rank));
    return newDeck;
}

---

in the booleanIsSameSuit implementation you say suit.equals(other.suit); how? Suitable doesn't know about suits?

Thats correct.

We have three options to solve this:

1. cast other to Card

   This works because we now that there is no other class in the program that implements the Suitable interface. But it is kinda brittle.

2. add a method getSuit() to the interface.

   The problem with that is that this method is not useful a user of a Suitable only for the implementer.

3. so the best is to also add a generic parameter to the interface:

---

interface Suitable<T> {
    booleanIsSameSuit(T other);
}
interface Rankable<T> {
   booleanIsConsecutive(T other);
}

---

public class Card implements Suitable<Card>, Rankable<Card>, Comparable<Card>{
  private final Rank rank;
  private final Suit suit;
  public Card(Suit suit, Rank rank){
     this.suit = suit;
     this.rank = rank;
  }

  @Override 
  public int compareTo(Card other){
    // compare rank and suit
  }

  @Override 
  public booleanIsSameSuit(Card other){
    return suit.equals(other.suit);
  }

  @Override 
  public booleanIsConsecutive(Card other){
    return 1+rank.ordinal()==other.rank.ordinal()
        ||  other.rank.ordinal()==rank.ordinal()-1;
  } 
}

Answer 2

Making a true generic implementation for a Generic Deck of Cards, is hard. Very hard. What kind of cards are you aiming to cover? Only standard playing cards? Or pictogram cards? Tarot cards? Uno cards? In this review I'm focusing mostly related to standard playing cards, as used for Black Jack.

Natural methods of interfaces

Which methods are natural to include in the interfaces, is yet another hard question, and I've got the following comments related to your interfaces:

• Have toString() in the interface – I'd say that being able to textually represent the various decks, hands and cards is needed.
• Equality/rank for cards and/or hands – Wouldn't it also be natural to allow for checking if cards are equal, or get rank value of cards? Similar reasoning also applies to the IHand and hands of cards.
• What about putting card backs in the IDeck?' – A lot of card games allows for cards to be put back into the deck, so that should maybe be part of the interface. Possibly also a reset method, used for starting over should belong in the IDeck?
• Type of IRank and ISuit?' – You've specified that these are int and String. Quite a few card implementations uses various variants of enums, which might be worth considering.
• Should ICard have methods related to rank and suit? – Questionable, but would it be natural to have some methods returning suit and rank?
• Dealing multiple cards? – Most card games I know allows for being dealt several cards, so maybe that belongs in the interface, aka ICard[] dealCard(int count).

More questions and/or ideas related to the interface could surely be given, but these was the one on top of my mind.

Comments to code

Some more specific comments to the Deck.java and BlackJack.java code:

• Ability to reset card deck – As briefly mentioned, I'm wondering if it wouldn't be good to reset the dealt cards, and so on. So maybe there should be some tracking of which cards have been dealt, instead of removing the cards completely from the list.

• Unclear initialization – When adding the cards you're using i % suits.length and i % ranks.length. If changing to five suits, or removing a rank, this would break the uniqueness of the combination.

  To me, a somewhat more reliable method to distribute between suits and ranks would be to use something like i / ranks.length and i % ranks.length.

  But yet another alternative, would be to use the passed arrays directly:

  for (String suit : suits) {
    for (int rank : ranks) {
      cards.add(new Card(suit, rank));
    }
  }
  

  This would be really flexible related to various combinations of suits and ranks.

• Use a static Random() – Here and there it is referred to making this a static instance variable, as it is somewhat expensive to instantiate this all the time. Supposedly.

• Strange ranks? – Kind of unusual ranks given in BlackJack class, and how does it handle that aces could be either 1 or 11? And isn't it normal to present the actual card used (that is, to show the Queen of Heart, but still let the value of it be 10?)

• Mostly nice formatting in rest of code – I've not commented too much on the general coding style, but it seems nice, clean and easy to read.