views:

144

answers:

2

Hello All,

I am attempting to create a simple program that analyzes the poker hands. Given n hands/players and the community cards (Texas hold 'em) I would like to determine the winner(s). However, my test is failing when I have two exact results - it only returns one winner. i.e The hand result contains J J 9 9 K, for both players, but my winners list contains one.

There are a couple reasons why I am posting here. Obviously, the first being is there anything apparent that is wrong here? Is this a good approach to have the sorting implemented (I couldn't see a reason to separate the sorting), is there a better approach and why?

I have a DetermineWinners method that is performing a order on the players' HandResult:

var ordered = _players.OrderByDescending(player => player.Result);
var bestHand = ordered.First();
var winners = ordered.Where(s => s.Result == bestHand.Result).ToList();

Here is my hand result class:

    public class HandResult : IComparable<HandResult>
{
    public Hand WholeCards { get; set; }
    public HandRanking HandRank { get; set; }
    public IEnumerable<Card> CommunityCards { get; set; }
    public IEnumerable<Card> UsedCards { get; set; }

    public static bool operator !=(HandResult a, HandResult b)
    {
        if (a == null)
            return b != null;
        if (b == null)
            return true;

        if (a.HandRank != b.HandRank)
            return true;

        //Compare Used Cards
        var aCards = a.UsedCards.Select(s => s.GetCardValue()).ToList();
        var bCards = b.UsedCards.Select(s => s.GetCardValue()).ToList();
        var cardGroup = a.HandRank.GetGrouping();
        for (int i = 0; i < 5; i += cardGroup[i])
        {
            if (aCards[i] != bCards[i])
                return true;
        }

        return false;
    }

    public static bool operator ==(HandResult a, HandResult b)
    {
        if ((object)a == null)
            return (object)b == null;
        if ((object)b == null)
            return false;

        if (a.HandRank != b.HandRank)
            return false;

        var aCards = a.UsedCards.Select(s => s.GetCardValue()).ToList();
        var bCards = b.UsedCards.Select(s => s.GetCardValue()).ToList();

        var cardGroup = a.HandRank.GetGrouping();

        for (int i = 0; i < 5; i += cardGroup[i])
        {
            if (aCards[i] != bCards[i])
            {
                return false;
            }
        }

        return true;
    }

    public static bool operator >(HandResult a, HandResult b)
    {
        if ((object)a == null)
            return (object)b != null;
        if ((object)b == null)
            return false;
        if ((object)a == (object)b)
            return false;

        if (a.HandRank != b.HandRank)
            return a.HandRank > b.HandRank;

        if (a == b)
            return false;

        var aCards = a.UsedCards.Select(s => s.GetCardValue()).ToList();
        var bCards = b.UsedCards.Select(s => s.GetCardValue()).ToList();

        var cardGroup = a.HandRank.GetGrouping();

        for (int i = 0; i < 5; i += cardGroup[i])
        {
            if (aCards[i] != bCards[i])
            {
                return aCards[i] > bCards[i];
            }
        }

        return false;
    }

    public static bool operator <(HandResult a, HandResult b)
    {
        if ((object)a == null)
            return (object)b == null;
        if ((object)b == null)
            return true;
        if ((object)a == (object)b)
            return false;

        if (a.HandRank != b.HandRank)
            return a.HandRank < b.HandRank;

        var aCards = a.UsedCards.Select(s => s.GetCardValue()).ToList();
        var bCards = b.UsedCards.Select(s => s.GetCardValue()).ToList();

        var cardGroup = a.HandRank.GetGrouping();

        for (int i = 0; i < 5; i += cardGroup[i])
        {
            if (aCards[i] != bCards[i])
                return aCards[i] < bCards[i];
        }

        return false;
    }

    #region IComparable<HandResult> Members

    public int CompareTo(HandResult other)
    {
        if (this == null)
            return other == null ? 0 : -1;
        if (other == null)
            return 1;

        if (this == other)
            return 0;
        if (this > other)
            return 1;

        return -1;
    }

    #endregion

    public override bool Equals(object obj)
    {
        return this == (HandResult)obj;
    }

    public override int GetHashCode()
    {
        var result = 0;

        result = (result * 397) ^ (int)this.HandRank;

        foreach (var card in this.UsedCards)
        {
            result = (result * 397) ^ card.GetHashCode();
        }

        return result;
    }
}

The GetCardResult method simply returns an integer representation of the card, i.e 1 through 14. Here is the HandRanking enum:

public enum HandRanking
{
    HighCard,
    Pair,
    TwoPair,
    ThreeOfAKind,
    Straight,
    Flush,
    FullHouse,
    FourOfAKind,
    StraightFlush,
    RoyalFlush
}

This is the GetGrouping extension on the HandRanking enum. It is used to help iterate through the cards when comparing values:

    internal static int[] GetGrouping(this HandRanking rank)
    {
        switch (rank)
        {
            case HandRanking.Pair:
                return new int[] { 2, 0, 1, 1, 1 };
            case HandRanking.TwoPair:
                return new int[] { 2, 0, 2, 0, 1 };
            case HandRanking.ThreeOfAKind:
                return new int[] { 3, 0, 0, 1, 1 };
            case HandRanking.FullHouse:
                return new int[] { 3, 0, 0, 2, 0 };
            case HandRanking.FourOfAKind:
                return new int[] { 4, 0, 0, 0, 1 };
            default:
                return new int[] { 1, 1, 1, 1, 1 };
        }
    }

Your help is greatly appreciated.

EDIT: My tests for CompareTo_Equal, CompareTo_LessThan and CompareTo_GreaterThan (which uses my operator overloads) succeed with results: 0, -1 and 1, respectively. I am lead to believe it's a problem with my Linq.OrderByDescending implementation. I would of thought this just uses the CompareTo implemenation, am I wrong?

+3  A: 

The comparison provided by IComparable (you should use the generic version IComparable<T> anyway) is solely represented through the interface method int CompareTo(IComparable other) whose result is

  • > 0 if the current object is greater than other
  • < 0 if the current object is less than other
  • = 0 for equality

The overloaded comparison operators don't matter for any code that relies on IComparable.

Dario
Thank you for the response! It seems I did not paste my code properly. I edited my post to reflect the full HandResult class, I have also used the Generic <T> version of IComparable. That said, my test is still failing.
Kurt
A: 

You are trying to select the Winners by choosing those with Results equal to the best hand's Result, but that is only true when the hands are exactly the same (I think, I'm having trouble seeing the whole picture without knowing what exactly is in cardGroup[i].

Are you certain the hands you expect to be identical are actually identical (as in same suit even)?

And then I would also suggest that you implement most of your operator overloads in terms of other operators. For example, != could be implemented as !(==) (inverting the result of calling the == operator, rather than reimplementing the whole set of conditionals - right now, if you change the way Equals is determined you'll have to duplicate those changes in !=)

qstarin
I see your original code was edited while I typed my answer, but I'm afraid I don't have time at the moment to review it and update my answer in turn.
qstarin
Appreciate the reply. Sorry about the code edit, it wasn't all there before. Yes, the winners are determined by the players with the bestHand. I have overriden the .Equals and == operator to check the values of the cards, so it excludes the suites.The cardGroup[i] section only hits when then HandRank is the same, in which case it will iterate through the values and make the comparison on the 5 cards that make the hand.
Kurt