I'm looking for an honest review of what I've got thus far. I'm still trying to get my head wrapped around software development and have picked Java as my starting language. That said, I figured that I would attempt to create a reusable module for which I'd use this as an external library.
I think it would be invaluable to hear from you experts on where I can improve or refactor. The code here is just snippets. Full code on GitHub.
Card.java
package com.frijolie.cards; public interface Card extends Comparable<Card> { // returns the value of the Card. inherently coupled with the Rank int getValue(); // returns the Rank of the Card. Values from Ace,2,3..., Jack,Queen,King Rank getRank(); // returns the Suit of the Card. Values are Diamonds, Hearts, Spades, Clubs Suit getSuit(); // returns the color of the Card. Values are Red or Black Color getColor(); // display the card onscreen void displayCard(); // compares this Card to another, returns if they are equal boolean equals(final Object o); // Generates a unique int value of a Card int hashCode(); // flips a Card faceUp or faceDown void flip(); // returns true if this Card is faceUp boolean isFaceUp(); } Rank.java
package com.frijlie.cards; public enum Rank { ACE("Ace","A",1), TWO("Two","2",2), THREE("Three","3",3), ... // snipped for brevity JACK("Jack","J",10), QUEEN("Queen","Q",10), KING("King","K","10); private final String name; private final String letter; private final int value; Rank(final String name, final String letter, final int value) { this.name = name; this.letter = letter; this.value = value; } ... // accessor methods } Suit.java
package com.frijolie.cards; public enum Suit { CLUBS("Clubs",'\u2663', Color.BLACK), DIAMONDS("Diamonds",'\u2666', Color.RED), SPADES("Spades",'\u2660', Color.BLACK), HEARTS("Hearts",'\u2764', Color.RED); private final String name; private final char symbol; private final Color color; Suit(final String name, final char symbol, final Color color) { this.name = name; this.symbol = symbol; this.color = color; } ... // accessor methods } Color.java
package com.frijolie.cards; public enum Color { RED("Red"), BLACK("Black"); private final String name; Color(final String name) { this.name = name; } public final String getName() { return name; } } Deck.java
package com.frijolie; // snipped imports public class Deck { private boolean isShuffled; private Stack<Card> deck; // default constructor, will create a single deck of 52 cards public Deck() { this(1) } // overloaded constructor, will create multiple decks of 52 cards public deck(int numberOfDecks) { deck = new Stack<>(); clearDeck(); for (int i = 0; i < numberOfDecks; i++) { populateDeck(); } } // a method to shuffle the collection of cards. default will shuffle 1x public void shuffle() { shuffle(1); } // overloaded shuffle method. will shuffle the deck a number of times. public void shuffle(int numberOfTimes) { isShuffled = true; for (int i = 0; i < numberOfTimes; i++) { Collections.shuffle(deck); } } // returns true if the deck has been shuffled public boolean isShuffled() { return isShuffled; } // returns the number of cards remaining in the deck public int numberOfCards() { return deck.size(); } // a method to remove a card from the deck public Card removeCard(Card card) { if(deck.contains(Objects.requireNonNull(card))) { int index = deck.indexOf(card); return deck.remove(index); } else { return null; } } // returns true if the specified card is contained in the deck public boolean containsCard(Card card) { return deck.contains(Objects.requireNonNull(card)); } // populates the deck with all possible cards in the deck. 52 cards public void populateDeck() { for(Suit suit: Suit.values()) { for(Rank rank: Rank.values()) { deck.add(new PlayingCard(rank,suit)); } } } // Overloaded method which allows any number of decks of 52 cards. public void populateDeck(int numberOfDecks) { for(int i = 0; i < numberOfDecks; i++) { populateDeck(); } } // remove all cards from the deck public void clearDeck() { deck.clear(); } // returns an unmodifiable collection, a view, of the deck. public Collection<Card> getUnmodifiableCollection() { return new ArrayList<>(deck); } // returns the first card in the collection. public Card pop() { return deck.pop(); } } Hand.java
package com.frijolie.cards; // snipped imports public class Hand implements Comparable<Hand> { private List<Card> cards; // default constructor public Hand() { cards = new ArrayList<>(); } // will add a specific card to the Hand public void addCard(Card card) { cards.add(Objects.requireNonNull(card)); } // will remove a specific card from the Hand if it's contained in the Hand public Card removeCard(Card card) { if (cards.contains(Objects.requireNonNull(card))) { int index = cards.indexOf(card); return cards.remove(index); } else { throw new IllegalArgumentException("This card did not exist in the hand. It cannot be removed."); } } // method to display all the Cards in the Hand public void displayHand() { for (Card card : cards) { if (card.isFaceUp()) { card.displayCard(); } else { System.out.println("[ Card Back ]"); } } } // returns the sum of the value of all Cards in the Hand public int calculateValue() { var handValue = 0; for (Card card : cards) { handValue += card.getRank().getValue(); } return handValue; } // remove all cards in the Hand public void clearHand() { cards.clear(); } // returns a unmodifiable collection, a view, of all cards in the Hand public Collection<Card> getUnmodifiableCollection() { return Collections.unmodifiableCollection(cards); } // compares one Hand to another public int compareTo(final Hand otherHand) { return this.calculateValue() - otherHand.calculateValue(); } // sorts cards in the Hand by Suit. Doesn't factor Rank within the Suit when sorting public void sortyBySuit() { cards.sort(Comparator.comparing(Card::getSuit)); } // sorts cards in the Hand by Rank. Doesn't factor Suit when sorting. public void sortByRank() { cards.sort(Comparator.comparing(Card::getRank)); } // sorts cards in the Hand by Color. Doesn't factor Suit or Rank when sorting. public void sortByColor() { cards.sort(Comparator.comparing(Card::getColor)); } // sorts cards in the Hand by Suit then Rank in ascending order. public void sort() { cards.sort((Card first, Card second) -> first.compareTo(second)); } // allows a card in the hand to be flipped in the opposite orientation public void flipCard(int index) { cards.get(index).flip(); } } PlayingCard.java
package com.frijolie.cards; import java.util.Objects; public class PlayingCard implements Card { private final Rank rank; private final Suit suit; private boolean isFaceUp; // default constructor public PlayingCard(final Rank rank, final Suit suit) { this.rank = rank; this.suit = suit; isFaceUp = true; } ... // snipping implemented accessor methods from Card interface // returns a string representation of the card @Override public String toString() { return rank.getLetter() + suit.getSymbol(); } // compares one PlayingCard to another @Override public boolean equals(final Object o) { Objects.requireNonNull(o); boolean isEqual; if (!(o instanceof Card)) { isEqual = false; } else if (this == 0) { isEqual = true; } else { var card = (Card) o; isEqual = (this.getRank().equals(card.getRank()) && this.suit.equals(card.getSuit())); } return isEqual; } // generates unique int value of a card @Override public int hashCode() { int suitMultiplier = suit.ordinal() + 2; int rankMultiplier = rank.ordinal() + 3; return (suitMultiplier * rankMultiplier) * 31; } // flip the card in the opposite orientation, faceUp/faceDown @Override public void flip() { isFaceUp = !isFaceUp; } // returns true if the card is faceUp @Override public boolean isFaceUp() { return isFaceUp; } // compares one card to another and returns an integer value @Override public int compareTo(Card o) { var card = Objects.requireNonNull(o); var suitCompareResult = this.suit.compareTo(card.getSuit()); if (suitCompareResult != 0) { return suitCompareResult; } else { return this.rank.compareTo(card.getRank()); } } } What could I improve?
I've toyed with the idea to make Hand and/or Deck an interface. However, I opted to go with classes in favor of code reuse and contained behavior. I send unmodifiable collections (Hand, Deck) as I want to control the behavior of the internal collection within their respective classes. You can see what it looks like, but you can't modify the Deck or Hand outside the class. Is this a good defensive strategy?
I'm also not 100% comfortable with the requirement of a Rank and Suit. How about a Magic the Gathering Card? That doesn't have a suit, but you could argue that it has a rank. Is it better to leave the implementation up to the other developer? Same could be said for an Uno card, Color.Red and Color.BLACK aren't the only options there.
I've thought about the Rank enum also. If the game you model has an ACE as a high card (not low as supplied here), how would you fix that in your own implementation? Create a new AceHighRank and declare them in a different order? However, that would break PlayingCard because it's expecting a Rank (not AceHighRank). Right?
