1
\$\begingroup\$

I decided to give this another go. Last attempt can be found here

This time around I tried my best to make this as generic as possible. The goal is to make this usable for any types of cards, not just traditional card-games.

AbstractOperation.java

package com.tn.deck;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;


public abstract class AbstractCardOperation<T extends Card> implements Receivable<T>, Sendable<T>  {
    protected List<T> cards;

    public AbstractCardOperation() {
        this.cards = new ArrayList<>();
    }

    public AbstractCardOperation(List<T> cards) {
        this.cards = cards;
    }

    public AbstractCardOperation(List<T> list, int size) {
        cards = IntStream.range(0, size)
                .mapToObj(i -> list)
                .flatMap(Collection::stream)
                .collect(Collectors.toList());
    }

    @Override
    public List<T> removeCards(int numberOfCards) {
        return IntStream.range(0, numberOfCards)
                .mapToObj(i -> cards.remove(i))
                .collect(Collectors.toList());
    }

    @Override
    public T removeCard() {
        if (cards.size() < 1) {
            throw new IllegalStateException("Deck is empty");
        }
        return cards.remove(0);
    }

    @Override
    public T removeCard(int index) {
        if (cards.size() - 1 < index) {
            throw new IllegalStateException("Index passed in is > size of deck-1");
        }
        return cards.remove(index);
    }

    @Override
    public void addCards(List<T> cards) {
        this.cards.addAll(cards);
    }

    @Override
    public void addCard(T card) {
        cards.add(card);
    }

    @Override
    public String toString() {
        return cards.toString();
    }
}

I know this has no abstract methods, and thus shouldn't be abstract, but it represent a lot of common functionality between Hand and Deck, and I don't want this to be instantiable. If I made an interface of this, I would have to implement all those functions twice.

Card.java

package com.tn.deck;


public interface Card<T>  {
    boolean isConsecutive(T other);
    boolean isEqual(T other);
    String toString();
}

Deck.java

package com.tn.deck;

import java.util.Comparator;
import java.util.List;


public interface Deck<T extends Card> {
    void shuffle();
    void sort(Comparator<T> sort);
    void addCardToPlayer(List<? super T> hand);
}

Hand.java

package com.tn.deck;

import java.util.List;


public interface Hand<T extends Card> {
   int calculate();
   void addCardToDeck(int index, List<? super T> deck);
   boolean isEqual(List<T> other);
   String toString();
}

Receivable.java

package com.tn.deck;

import java.util.List;


interface Receivable<T extends Card> {
    void addCards(List<T> cards);
    void addCard(T card);
}

Sendable.java

package com.tn.deck;

import java.util.List;


interface Sendable<T extends Card> {
    List<T> removeCards(int numberOfCards);
    T removeCard();
    T removeCard(int index);
}

Here is a package with some classes where I implement the interfaces:

BlackjackCard.java

package com.tn.test;

import com.tn.deck.Card;


public class BlackjackCard implements Card<BlackjackCard> {
    private Suit suit;
    private Rank rank;

    public BlackjackCard(Suit suit, Rank rank) {
        this.suit = suit;
        this.rank = rank;
    }

    public Suit getSuit() {
        return suit;
    }

    public Rank getRank() {
        return rank;
    }

    @Override
    public boolean isConsecutive(BlackjackCard other) {
        return Math.abs(rank.getValue() - other.rank.getValue()) == 1;
    }

    @Override
    public boolean isEqual(BlackjackCard other) {
        return suit.equals(other.suit) && rank.equals(other.rank);
    }

    @Override
    public String toString() {
        return suit.getIcon() + "" + rank.getName() + " ";
    }
}

BlackjackDeck.java

package com.tn.test;

import com.tn.deck.AbstractCardOperation;
import com.tn.deck.Card;
import com.tn.deck.Deck;

import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;


public class BlackjackDeck extends AbstractCardOperation<BlackjackCard> implements Deck<BlackjackCard> {

    public BlackjackDeck(List<BlackjackCard> deck) {
        super(deck);
    }

    public BlackjackDeck(List<BlackjackCard> deck, int numberOfDecks) {
        super(deck, numberOfDecks);
    }

    public List<BlackjackCard> getDeck() {
        return cards;
    }

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

    @Override
    public void sort(Comparator<BlackjackCard> sortable) {
        cards.sort(sortable);
    }

    @Override
    public void addCardToPlayer(List<? super BlackjackCard> hand) {
        hand.add(cards.remove(0));
    }

    @SafeVarargs
    public static <T extends Card> void print(Deck<T>... decks) {
        Arrays.stream(decks).forEach(deck -> System.out.println(deck.toString()));
    }
}

Player.java

package com.tn.test;

import java.util.List;


public class Player {
    private PlayerHand hand;

    public Player(PlayerHand hand) {
        this.hand = hand;
    }

    public List<BlackjackCard> getHand() {
        return hand.getHand();
    }

    public void addCardToDeck(int index, List<? super BlackjackCard> deck) {
        hand.addCardToDeck(index, deck);
    }

    public boolean compareHand(Player other) {
        return hand.isEqual(other.hand.getHand());
    }

    public String toString() {
        return hand.toString();
    }
}

PlayerHand.java

package com.tn.test;

import com.tn.deck.AbstractCardOperation;
import com.tn.deck.Hand;

import java.util.Collections;
import java.util.List;


public class PlayerHand extends AbstractCardOperation<BlackjackCard> implements Hand<BlackjackCard> {

    public PlayerHand(List<BlackjackCard> hand) {
        super(hand);
    }

    List<BlackjackCard> getHand() {
        return cards;
    }

    @Override
    public int calculate() {
        return 0;
    }

    @Override
    public void addCardToDeck(int index, List<? super BlackjackCard> deck) {
        deck.add(cards.remove(index));
    }

    @Override
    public boolean isEqual(List<BlackjackCard> other) {
        return !Collections.disjoint(cards, other);
    }
}

Rank.java

package com.tn.test;


public enum Rank {
    TWO("2", 2), THREE("3", 3), FOUR("4", 4), FIVE("5", 5),
    SIX("6", 6), SEVEN("7", 7), EIGHT("8", 8), NINE("9", 9), TEN("10", 10),
    JACK("J", 10), QUEEN("Q", 10), KING("K", 10), ACE("A", 11);

    private final String name;
    private final int value;

    Rank(String name, int value) {
        this.name = name;
        this.value = value;
    }

    String getName() {
        return name;
    }

    int getValue() {
        return value;
    }
}

Suit.java

package com.tn.test;


public enum Suit {
    SPADE("\u2660"),
    HEART("\u2665"),
    DIAMOND("\u2666"),
    CLUB("\u2663");

    private final String icon;

    Suit(String icon) {
        this.icon = icon;
    }

    public String getIcon() {
        return icon;
    }
}

And lastly, my Main where I instantiate some of the classes, and test a few methods.

Yes - this should have been unit-tests, and I will implement that as well.

Main.java

import com.tn.test.*;

import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class Main {

    public static void main(String[] args) {
        List<BlackjackCard> blackjackCards = Arrays.stream(Suit.values())
                .flatMap(suit -> Arrays.stream(Rank.values()).map(rank -> new BlackjackCard(suit, rank)))
                .collect(Collectors.toList());

        BlackjackDeck deck = new BlackjackDeck(blackjackCards);
        deck.shuffle();

        Player[] players = IntStream.rangeClosed(1, 2)
                .mapToObj(player -> new Player(new PlayerHand(deck.removeCards(5))))
                .toArray(Player[]::new);

        Arrays.stream(players).forEach(player -> System.out.println(player.toString()));

        deck.sort(Comparator
                .comparing(BlackjackCard::getRank)
                .thenComparing(BlackjackCard::getSuit));

        BlackjackDeck.print(deck);


        deck.addCardToPlayer(players[0].getHand());
        System.out.println(players[0].toString());
        players[0].addCardToDeck(4, deck.getDeck());
        System.out.println(players[0].toString());

        BlackjackDeck.print(deck);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Why do you have a Hand.isEqual(List<T> other). Shouldn't the argument be a Hand as well and isn't overriding Object.equals sufficient? \$\endgroup\$ – Karl Richter May 27 '17 at 10:06
  • \$\begingroup\$ @KarlRichter yeah - you're right. But I do want it in the interface to ensure that it will be implemented. \$\endgroup\$ – Nilzone- May 27 '17 at 11:40
  • \$\begingroup\$ This is very high quality code. Every part you introduced and I found was expected to be found by me. And every part I looked at had the semantic and the implementation I expected. There are no surprises and no complicated micromanagement. Everythings has its place where it belongs to. \$\endgroup\$ – oopexpert Jun 3 '17 at 13:46
  • \$\begingroup\$ @oopexpert thank you very much :) Only thing I don't like is that Player inherits from the AbtractClass. In my mind that is not a IS-A relationship. \$\endgroup\$ – Nilzone- Jun 3 '17 at 15:02
2
\$\begingroup\$

I'm going into details here but I hope this is still helpful:

  • I don't see the need for the Player class as it only forwards to the PlayerHand

  • Receivable and Sendable are not used

  • Player.compareHand should be named isEqual for consistency

  • I personally prefer having null checks in equals methods to ensure nothing breaks if null is passed as argument

  • AbstractOperation.removeCards: It might look more elegant the way you have implemented it but creating a new ArrayList every time this method gets called is quite performance costy. Simply iterating of the list and removing one by one causes less overhead.

  • Adding the toString method to an interface does not do anything. You can not force a class to implement it as every class already has the default Object implementation.

\$\endgroup\$
  • \$\begingroup\$ Good points - thank you. Just one thing. Receivable and Sendable is used - in the AbstractOperation class \$\endgroup\$ – Nilzone- May 27 '17 at 16:11

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.