nosuchblogger

One of the reasons for keeping the issues log is so I don't have to remember all the little things that crop up and (probably) need refactoring at some point. And as I've just completed the last feature, now is a good time.

PokerRound is the problem

It happens to be that most of the points are all really about one thing: the PokerRound class isn't modelling the cards correctly, which has a detrimental effect on other parts of the application. It's therefore a simple start: let's introduce a HoleCards class;

class HoleCards {
  private final Card holeCard1;
  private final Card holeCard2;
  public HoleCards(Card holeCard1, Card holeCard2) {
    this.holeCard1 = holeCard1;
    this.holeCard2 = holeCard2;
  }
  public Card getHoleCard1() { return holeCard1; }
  public Card getHoleCard2() { return holeCard2; }
}

Simple enough. I take small steps in the PokerRound class itself. At this point, I do two things:

1. Add a new map of String and HoleCards. This will populate with players and their initial two cards.
2. Modify the deal method by adding the two hole cards as arguments. I can then remove the holeCards method.

Note, at this point, I haven't modified the rest of the PokerRound implementation to use the new map.

Slowly does it

The next thing we're missing is the concept of the community cards. Currently, all the cards are simply stored against each player. I decide to introduce a Deal class and PokerRound will contain a list of Deals (i.e. three deals: flop; turn; river):

class Deal {
  private final List<Card> cardsInDeal;
  private final Set<String> playersFolded = new HashSet<String>();
  public Deal(List<Card> cards) {
    this.cardsInDeal = cards;
  }
  public List<Card> getCardsInDeal() { return this.cardsInDeal; }
  public void fold(String player) { this.playersFolded.add(player); }
  public boolean folded(String player) { return this.playersFolded.contains(player); }
  public Set<String> getPlayersFolded() { return this.playersFolded; }
}

As you can see, each deal also contains the player(s) that folded during that deal. It seems better in the Deal class than in the PokerRound class - at the moment at least!

Again, PokerRound is modified appropriately:

1. Added a new list of Deal objects.
2. Changed the deal methods to add to this list.

All the tests still pass; unsurprising really as nothing has really changed - the new map of players to HoleCards and list of Deals aren't being read as yet. That happens next.

Problem spotting

The main change is within the results method. This iterates over all the players, writing out all their cards, and if they folded or won. I spot a problem immediately. All I have is a list of Deals and a map. Iterating over a map doesn't provide a reliable ordering - that is when I add the players as the initial deal method is called, when iterating, I'm unlikely to return the players in the same order. Even if it did happen for the tests, this shouldn't be relied upon.

However, the PokerGame class does have the list of players, and this is probably the order in which one would expect the results to be printed. Except the PokerRound class doesn't know about the game. I think it's reasonable to add this dependency - a round is part of a game, so I add a constructor to take a PokerGame as an argument.

The results method then uses the collection of players from the game as the iteration order.

The remaining changes are all relatively straight forward after this. The largest change being in the RoundWinners class, which now takes the map of players and HoleCards and list of deals instead of the prior map of players and all the cards.

Are we done yet?

All the tests are green. I update the issues log, sit back and am happy enough with the improvement.

Next time, I'll be looking at making some quick progress on the remaining hand identifiers - starting from two-pairs and working my way up.