4
\$\begingroup\$

This is my second solution to Project Euler #54. You can find the original post here.

Brief problem summary:

The file, poker.txt, contains one-thousand random hands dealt to two players. Each line of the file contains ten cards (separated by a single space): the first five are Player 1's cards and the last five are Player 2's cards. You can assume that all hands are valid (no invalid characters or repeated cards), each player's hand is in no specific order, and in each hand there is a clear winner.

How many hands does Player 1 win?

Note: You can specify the data file with the option -DDATA_FILE=<your classpath location>. The default value is "/data/p054_poker.txt".

Changes:

  • All of the changes in @janos's answer, except that I did not add a space after if and for statements because I don't mind that, and I left inlined if and single-letter statements in a few places (though I did change several of them).
  • A number of variable and method renamings (not limited to the ones that @janos suggested)
  • Fixed the straight / straight-flush bug in @barq's answer
  • Refactored redundant code into methods
  • Created a lot of small helper methods to make lines more legible and shorter
  • Added support for wheel straights (A-2-3-4-5)
  • Refactored logic for detecting Pair / Two Pair / Trips / Full House / Quads into its own inner class (CardMatchRankingBuilder, in Hand.java)
  • Standardized ranking methodology for Straights / Flushes / Straight-Flushes

Card.java

package problem54;

public class Card implements Comparable<Card> {
    private final Value value;
    private final Suit suit;

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

    public Card(String s) {
        this(Value.of(s.charAt(0)), Suit.of(s.charAt(1)));
    }

    public Value getValue() {
        return value;
    }

    public Suit getSuit() {
        return suit;
    }

    @Override
    public int compareTo(Card o) {
        return value.compareTo(o.value);
    }

    @Override
    public String toString() {
        return String.format("%s%s", value.getValueChar(), suit.getSuitChar());
    }
}

Hand.java

package problem54;

import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;

public class Hand implements Comparable<Hand> {
    private final List<Card> cards;
    private final List<Card> cardsSentinel;
    private final ValueRanking ranking;

    public Hand(List<Card> cards) {
        this.cards = Collections.unmodifiableList(new ArrayList<>(cards));

        List<Card> temp = new ArrayList<>(cards);
        temp.add(new Card(Value.NULL, Suit.NULL));
        Collections.sort(temp);
        this.cardsSentinel = Collections.unmodifiableList(temp);

        ValueRanking straightRanking = straightRanking();

        if(straightRanking != null) {
            ranking = straightRanking;
        } else {
            ranking = cardMatchRanking();
        }
    }

    public List<Card> getCards() {
        return cards;
    }

    public ValueRanking getRanking() {
        return ranking;
    }

    @Override
    public int compareTo(Hand o) {
        return ranking.compareTo(o.ranking);
    }

    private ValueRanking straightRanking() {
        boolean straight = isStraight();
        boolean flush = isFlush();
        if(straight && flush) {
            return makeRanking(Ranking.STRAIGHT_FLUSH);
        } else if(flush) {
            return makeRanking(Ranking.FLUSH);
        } else if(straight) {
            return makeRanking(Ranking.STRAIGHT);
        } else {
            return null;
        }
    }

    private ValueRanking cardMatchRanking() {
        CardMatchRankingBuilder builder = new CardMatchRankingBuilder();

        for(Card c : cardsSentinel) {
            builder.addCard(c.getValue());
        }

        return builder.build();
    }

    private boolean isFlush() {
        for(int i = 0; i < 4; i++) {
            if(cardSuit(i) != cardSuit(i+1)) {
                return false;
            }
        }
        return true;
    }

    private boolean isStraight() {
        for(int i = 0; i < 4; i++) {
            if(cardRank(i) + 1 != cardRank(i+1)) {
                if(isWheel(i)) return true;
                return false;
            }
        }
        return true;
    }

    private boolean isWheel(int cardIndex) {
        return cardIndex == 3 
            && cardValue(cardIndex) == Value.FIVE
            && cardValue(cardIndex + 1) == Value.ACE;
    }

    private Value cardValue(int cardIndex) {
        return cardsSentinel.get(cardIndex).getValue();
    }

    private Suit cardSuit(int cardIndex) {
        return cardsSentinel.get(cardIndex).getSuit();
    }

    private int cardRank(int cardIndex) {
        return cardValue(cardIndex).ordinal();
    }

    private ValueRanking makeRanking(Ranking type) {
        return new ValueRanking(type, cardValue(4), (Value) null, getKickers());
    }

    private List<Value> getKickers() {
        List<Value> kickers = new LinkedList<Value>();
        for(int i = 0; i < 4; i++) {
            kickers.add(cardValue(i));
        }
        return kickers;
    }

    private static class CardMatchRankingBuilder {
        private Value previous = null;
        private int counter = 0;
        private int pair = 0;
        private boolean trips, quads;

        private Value primary = null;
        private Value secondary = null;
        private List<Value> kicker = new ArrayList<>(5);

        public void addCard(Value newCard) {
            if(pairComplete(newCard)) {
                handleCardGroup(newCard);
                resetCounter(newCard);
            } else {
                counter++;
            }
        }

        private void handleCardGroup(Value newCard) {
            switch(counter) {
            case 2:
                handlePair(newCard);
                break;
            case 3:
                handleTrips(newCard);
                break;
            case 4:
                handleQuads(newCard);
                break;
            default:
                addKicker();
            }
        }

        private boolean pairComplete(Value newCard) {
            return previous != newCard;
        }

        private void resetCounter(Value newCard) {
            previous = newCard;
            counter = 1;
        }

        private void handlePair(Value current) {
            pair++;
            if(primary == null) {
                primary = previous;
            } else if(pairHasLowerRanking(current)) {
                secondary = current;
            } else {
                demotePrimary();
            }
        }

        private boolean pairHasLowerRanking(Value current) {
            return trips || primary.compareTo(current) > 0;
        }

        private void demotePrimary() {
            secondary = primary;
            primary = previous;
        }

        private void handleTrips(Value current) {
            trips = true;
            demotePrimary();
        }

        private void handleQuads(Value current) {
            quads = true;
            primary = previous;
        }

        private void addKicker() {
            if(previous != null) kicker.add(previous);
        }

        public ValueRanking build() {
            if(quads) {
                return makeRanking(Ranking.QUADS);
            } else if(trips) {
                if(pair > 0) {
                    return makeRanking(Ranking.FULL_HOUSE);
                } else {
                    return makeRanking(Ranking.TRIPS);
                }
            } else if(pair == 2) {
                return makeRanking(Ranking.TWO_PAIR);
            } else if(pair == 1) {
                return makeRanking(Ranking.PAIR);
            } else {
                return makeRanking(Ranking.HIGH_CARD);
            }
        }

        private ValueRanking makeRanking(Ranking type) {
            return new ValueRanking(type, primary, secondary, kicker);
        }
    }
}

Parser.java

package problem54;

import java.io.BufferedReader;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.List;

public class Parser implements Closeable {
    private final BufferedReader reader;

    public Parser(InputStream in) {
        this.reader = new BufferedReader(new InputStreamReader(in));
    }

    public Round getNextRound() throws IOException {
        String nextLine = reader.readLine();
        String[] cardStrings = nextLine.split(" ");

        List<Card> first = makeCardList(cardStrings, 0, 5);
        List<Card> second = makeCardList(cardStrings, 5, 10);

        return new Round(first, second);
    }

    public static List<Card> makeCardList(String[] cardStrings, int start, int stop) {
        List<Card> cardList = new ArrayList<>(stop - start);
        for(int i = start; i < stop; i++) {
            cardList.add(new Card(cardStrings[i]));
        }
        return cardList;
    }

    @Override
    public void close() throws IOException {
        reader.close();
    }

    public boolean hasNext() throws IOException {
        return reader.ready();
    }
}

Problem.java

package problem54;

import java.io.IOException;
import java.io.InputStream;

public class Problem {
    public static void main(String... args) throws IOException {
        InputStream is = loadDataFileStream();
        Parser p = new Parser(is);

        int counter = countPlayerOneWinners(p);

        System.out.println(counter);

        p.close();
    }

    private static int countPlayerOneWinners(Parser p) throws IOException {
        int counter = 0;
        while(p.hasNext()) {
            Round r = p.getNextRound();
            if(r.playerOneWins()) counter++;
        }
        return counter;
    }

    private static InputStream loadDataFileStream() {
        String dataFile = System.getProperty("DATA_FILE");
        if(dataFile == null) dataFile = "/data/p054_poker.txt";
        InputStream is = Problem.class.getResourceAsStream("/data/p054_poker.txt");
        return is;
    }
}

Ranking.java

package problem54;

public enum Ranking {
    HIGH_CARD,
    PAIR,
    TWO_PAIR,
    TRIPS,
    STRAIGHT,
    FLUSH,
    FULL_HOUSE,
    QUADS,
    STRAIGHT_FLUSH,
    ROYAL_FLUSH;
}

Round.java

package problem54;

import java.util.List;

public class Round {
    public final Hand playerOne;
    public final Hand playerTwo;

    public Round(List<Card> firstCards, List<Card> secondCards) {
        this.playerOne = new Hand(firstCards);
        this.playerTwo = new Hand(secondCards);
    }

    public boolean playerOneWins() {
        return playerOne.compareTo(playerTwo) > 0;
    }
}

Suit.java

package problem54;

import java.util.HashMap;
import java.util.Map;

public enum Suit {
    HEART ('H'),
    DIAMOND ('D'),
    SPADE ('S'),
    CLUB ('C'),
    NULL ('x');

    private final char suitChar;

    private static final Map<Character, Suit> valueMap = new HashMap<>();

    static {
        for(Suit c : Suit.values()) {
            valueMap.put(c.getSuitChar(), c);
        }
    }

    private Suit(char suitChar) {
        this.suitChar = suitChar;
    }

    public char getSuitChar() {
        return suitChar;
    }

    public static Suit of(char value) {
        Suit s = valueMap.get(value);
        return s == null ? NULL: s;
    }
}

Value.java

package problem54;

import java.util.HashMap;
import java.util.Map;

public enum Value {
    TWO ('2'),
    THREE ('3'),
    FOUR ('4'),
    FIVE ('5'),
    SIX ('6'),
    SEVEN ('7'),
    EIGHT ('8'),
    NINE ('9'),
    TEN ('T'),
    JACK ('J'),
    QUEEN ('Q'),
    KING ('K'),
    ACE ('A'),
    NULL ('x');

    private final char valueChar;

    private static final Map<Character, Value> valueMap = new HashMap<>();

    static {
        for(Value c : Value.values()) {
            valueMap.put(c.getValueChar(), c);
        }
    }

    private Value(char cardString) {
        this.valueChar = cardString;
    }

    public char getValueChar() {
        return valueChar;
    }

    public static Value of(char value) {
        Value v = valueMap.get(value);
        return v == null ? NULL : v;
    }
}

ValueRanking.java

package problem54;

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

public class ValueRanking implements Comparable<ValueRanking> {
    private final Ranking rank;
    private final Value primary;
    private final Value secondary;
    private final List<Value> kicker;

    public ValueRanking(Ranking rank, Value primary, Value secondary, List<Value> kicker) {
        this.rank = rank;
        this.primary = primary == null ? Value.NULL : primary;
        this.secondary = secondary == null ? Value.NULL : secondary;
        List<Value> kickerTemp = new ArrayList<>(kicker);
        Collections.sort(kickerTemp);
        Collections.reverse(kickerTemp);
        this.kicker = Collections.unmodifiableList(kickerTemp);
    }

    public Ranking getRank() {
        return rank;
    }

    public Value getPrimary() {
        return primary;
    }

    public Value getSecondary() {
        return secondary;
    }

    public List<Value> getKicker() {
        return kicker;
    }

    @Override
    public int compareTo(ValueRanking o) {
        int compareTo = rank.compareTo(o.rank);
        if(compareTo != 0) return compareTo;

        compareTo = primary.compareTo(o.primary);
        if(compareTo != 0) return compareTo;

        compareTo = secondary.compareTo(o.secondary);
        if(compareTo != 0) return compareTo;

        for(int i = 0; i < kicker.size(); i++) {
            compareTo = kicker.get(i).compareTo(o.kicker.get(i));
            if(compareTo != 0) return compareTo;
        }
        return 0;
    }

    @Override
    public String toString() {
        return String.format(
                "[%s,%s,%s,%s]",
                rank, primary, secondary, kicker);
    }
}
\$\endgroup\$
2
\$\begingroup\$

Your code is neat, and structured, and the OO nature appears to be clear.

Some other issues I have with it are more boilerplate types.

Comparable

Comparable has a contract:

The natural ordering for a class C is said to be consistent with equals if and only if e1.compareTo(e2) == 0 has the same boolean value as e1.equals(e2) for every e1 and e2 of class C

In practice, what this means is that in every case where you implement Comparable, you also need to override the equals and hashCode() methods. You implement Comarable in ValueRanking, Hand and Card. Each of these need to be changed in order for the consistency of the contract to be maintained.

Note that the contract allows some tricks, for example, equals becomes:

public boolean equals(Object other) {
    return other instanceof Hand && compareTo((Hand)other) == 0;
}

Hashcode is a bit more complicated though.

By having a correct implementation for these you can then rely on using them as keys in Maps, etc. It is good practice to implement things correctly.

In your case, as you know, this will not affect the results of your program, but it will impact the reusability of it.

Enums

The null placeholder positions x in the Suit and Value enums are out of place, and should not exist. What is their purpose?

Value Look-ups

Some alternative suggestions....

In your Value Enum you have a Map to provide look-ups between characters and Values. The code is:

private static final Map<Character, Value> valueMap = new HashMap<>();

static {
    for(Value c : Value.values()) {
        valueMap.put(c.getValueChar(), c);
    }
}

public static Value of(char value) {
    Value v = valueMap.get(value);
    return v == null ? NULL : v;
}

I would consider a simpler, and probably faster way of doing it:

private static final Value[] valueMap = new Value[128];

static {
    for(Value c : Value.values()) {
        valueMap[c.getValueChar()] = c;
    }
}

public static Value of(char value) {
    Value ret = value < valueMap.length ? valueMap[value] : null;
    if (ret == null) {
        throw new IllegalArguementException("Illegal value char '" + value + "'.");
    }
    return ret;
}

The above code does not have the overhead of the char -> Character translation and all the map overheads.

On further thought, a switch statement, though long-winded, would probably be the best (performance wise):

switch (value) {
    case 'A' : return ACE;
    case 'K' : return KING;
    ....
}

I would normally add a newline after the : in the case statement, but in a situation like that where all cases are identical, I may get lazy for the sake of consistency...

The same thing can be done for Suit

makeCardList

This function is used as follows:

String[] cardStrings = nextLine.split(" ");

List<Card> first = makeCardList(cardStrings, 0, 5);
List<Card> second = makeCardList(cardStrings, 5, 10);

and

public static List<Card> makeCardList(String[] cardStrings, int start, int stop) {
    List<Card> cardList = new ArrayList<>(stop - start);
    for(int i = start; i < stop; i++) {
        cardList.add(new Card(cardStrings[i]));
    }
    return cardList;
}

This code can be simplified with the Java List.subList() function, and some neat Java 8 functions:

List<Card> cardList = Stream.of(nextLine.split(" "))
    .map(c -> new Card(c))
    .collect(Collectors.toList());
List<Card> first = cardList.subList(0, 5);
List<Card> second = cardList.subList(5, 10);
\$\endgroup\$
  • 1
    \$\begingroup\$ Great answer! Just FYI this code is specifically Java 7. \$\endgroup\$ – durron597 Feb 18 '15 at 18:28

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.