Deck of cards implementation

Question

Update: Simple Blackjack implementation

I was playing Blackjack in a phone app and decided to try to do the game in Java, using the console for a start. So far I'd like to review the deck of cards implementation. I decided to use an OOP approach with the classes Value, Suit, Card and Deck.

First of all, I'd like to get some feedback and pieces of advice about how I decided to structure my code. Is there anything wrong? Should something be done differently?

import java.util.Stack;
import java.util.Random;
import java.util.Collections;

public class Blackjack {

    public static String capitalizeFully(String str) {
        String s = new String();

        for (int i = 1; i < str.length(); i++) {
            s += Character.toLowerCase(str.charAt(i));
        }

        return Character.toUpperCase(str.charAt(0)) + s;
    }

    public static class Value {

        private int value;
        private static final String[] string_values = {
            "Ace",
            "Two",
            "Three",
            "Four",
            "Five",
            "Six",
            "Seven",
            "Eight",
            "Nine",
            "Ten",
            "Jack",
            "Queen",
            "King"
        };

        public Value(int value) throws IllegalArgumentException {
            if (value < 1 || value > 13) {
                throw new IllegalArgumentException("Invalid card value (must be between 1 and 13).");
            }
            this.value = value;
        }

        public Value(String value) throws IllegalArgumentException {
            int i = 0;
            String val = capitalizeFully(value);

            while (i < string_values.length && !val.equals(string_values[i])) {
                i++;
            }

            if (i == string_values.length) {
                throw new IllegalArgumentException("Invalid card value.");
            } else {
                this.value = i + 1;
            }
        }

        public String toString() {
            return string_values[value - 1];
        }
    }

    public static class Suit {

        private String name;
        private static final String[] string_names = {
            "Hearts",
            "Diamonds",
            "Clubs",
            "Spades"
        };

        public Suit(String name) throws IllegalArgumentException {
            int i = 0;
            String n = capitalizeFully(name);

            while (i < string_names.length && !n.equals(string_names[i])) {
                i++;
            }

            if (i == string_names.length) {
                throw new IllegalArgumentException("Invalid suit name.");
            } else {
                this.name = n;
            }
        }

        public String toString() {
            return name;
        }
    }

    public static class Card {

        private Value value;
        private Suit suit;

        public Card(Value value, Suit suit) {
            this.value = value;
            this.suit = suit;
        }

        public Card(int value, Suit suit) {
            this.value = new Value(value);
            this.suit = suit;
        }

        public Card(String value, Suit suit) {
            this.value = new Value(value);
            this.suit = suit;
        }

        public Card(Value value, String suit) {
            this.value = value;
            this.suit = new Suit(suit);
        }

        public Card(int value, String suit) {
            this.value = new Value(value);
            this.suit = new Suit(suit);
        }

        public Card(String value, String suit) {
            this.value = new Value(value);
            this.suit = new Suit(suit);
        }

        public String toString() {
            String str = value + " of " + suit;
            return str;
        }
    }

    public static class Deck {

        private Stack<Card> deck;
        private static final int number = 52;

        public Deck() {
            deck = new Stack<Card>();
            Suit hearts, diamonds, clubs, spades;

            hearts = new Suit("hearts");
            diamonds = new Suit("diamonds");
            clubs = new Suit("clubs");
            spades = new Suit("spades");

            Suit[] suits = { hearts, diamonds, clubs, spades };

            for (int i = 0; i < suits.length; i++) {
                for (int j = 1; j <= 13; j++) {
                    deck.push(new Card(j, suits[i]));
                }
            }
        }

        public void shuffle() {
            long seed = System.nanoTime();
            Collections.shuffle(deck, new Random(seed));
        }

        public void add(Card card) {
            deck.push(card);
        }

        public Card draw() {
            return deck.pop();
        } 

        public String toString() {
            String str = new String();
            for (int i = 0; i < deck.size(); i++) {
                str += deck.get(i) + "\n";
            }
            return str;
        }
    }

    public static <T> void p(T output) {
        System.out.println(output);
    }

    public static void main(String[] args) {

        Deck deck = new Deck();
        deck.shuffle();
        p(deck);
    }
}

However, this is an implementation for just the 52-card standard deck. How about different decks, with different suits and a different range of values? I thought maybe it'd be a good idea that instead of hardcoding these attributes, I could make an alternative implementation where the user can define his/her own set of suits and range of values, so the deck is constructed given these set of choices.

For example:

Value[] values = { new Value("As", 1), new Value("Dos", 2), ..., new Value("Rey", 12) };
Suit[] suits = { new Suit("Oros"), new Suit("Sotas"), new Suit("Espadas"), new Suit("Bastos") };

Deck spanish_deck = new Deck(suits, values);

Does this second alternative make sense? Is it a good approach?

Update: Simple Blackjack implementation

Answer 1

For starters, using Java enums will likely save you a lot of the boilerplate-like template code you have for Value and Suit.

Your Card constructors seem to be over-engineered too... what you have are just permutations of ints, Strings and objects arguments... I suggest you pick one and stick with that, preferably just Value and Suit. If your Card objects are meant to be immutable, I'll suggest saving its String representation (used, naturally, in toString()) as an instance field first. In fact, you should probably go a step further and override the equals() and hashCode() methods too to simplify/standardize such usage for the class. This will be helpful if you are using them in most, if not all, Collection classes.

Unfortunately, your implementation for capitalizeFully() doesn't seem too good... First, I'm not sure what it's meant by 'fully' capitalizing a String. I thought you meant converting a String to uppercase. Turns out you are just capitalizing the first letter. So why not call that then?

private static String capitalizeFirstLetter(String input) {
    // ...
}

Unless you are constructing Strings from byte/char arrays, with an optional Charset, you should (almost) never need to instantiate a String by doing new String(). Then, using the toLowerCase() and toUpperCase() methods...

private static String capitalizeFirstLetter(String input) {
    // handle null or "" Strings if required
    return input.substring(0, 1).toUpperCase() + input.substring(1).toLowerCase();
}

If you want to cater for Locale-specific conversions, there are methods for that too.

Last, you need a much better name for your print (?) p() method... what is it supposed to do actually? The returned object doesn't appear to be used in any way.

edit further explanation for an enum-approach

Say you have a Rank and Suit enum:

enum Rank {
    ONE,
    TWO,
    // ...
    JACK,
    QUEEN,
    KING,
    ACE; // opt-ing to 'rank' ACE as the highest

    public int getValue() {
        return ordinal() + 1;
    }
}

enum Suit {
    CLUB,
    DIAMOND,
    HEART,
    SPADE;
}

Some benefits of using enums are:

• Values are Comparable to each other.
• Efficient use in Maps and Sets via EnumMap and EnumSet classes, when required.
• built-in toString() representation.
• built-in index-based lookup for values via ordinal().
• built-in String-based lookup for values via valueOf(String).
• safe ==-based comparison.

For your Card class, you can then do something like:

// note: final modifier to indicate this class cannot be subclassed
// in order to preserve the immutable features
public final class Card {
    private final Rank rank;
    private final Suit suit;
    private final String toString;
    private final int hashCode;

    public Card(Rank rank, Suit suit) {
        this.rank = rank;
        this.suit = suit;
        this.toString = getToString(rank, suit);
        // Objects.hash() is from Java 7
        this.hashCode = Objects.hash(rank, suit);
    }

    private static String getToString(Rank rank, Suit suit) {
        // code review comment: will return e.g. "Seven of Hearts"
        return String.format("%s of %ss",
                    capitalizeFirstLetter(rank.toString()),
                    capitalizeFirstLetter(suit.toString()));
    }

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

    @Override
    public int hashCode() {
        return hashCode;
    }

    @Override
    public boolean equals(Object o) {
        if (!(o instanceof Card)) {
            return false;
        }
        Card otherCard = (Card) o;
        return otherCard.rank == rank && otherCard.suit == suit;
    }
}

I hope this kind of illustrates that your Value/Rank, Suit and Card classes can have similar functionality with less boilerplate-like template code... The idea behind pre-computing the String representation and hash code first is to avoid repeated computations, should the methods be called frequently. This works especially well for immutable classes like the Card class as shown.