7
\$\begingroup\$

Is there a better, more elegant solution to the following method?

Boolean[] spades = new Boolean[10]; // 40 cards deck

foreach (Card card in m_Cards.Where(card => (card.Suit == Suit.Spades)))
    spades[(Int32)card.Rank] = true; // Rank goes from Ace (0) to King (10)

if (spades[0] && spades[1] && spades[2])
{
    points += 3;

    for (Int32 i = 3; i < spades.Length; ++i)
    {
        if (spades[i])
            ++points;
        else
            break;
    }
}

In fact, the function awards 3 points to the player only if he owns Ace, Two and Three... and, always if this condition is satisfied, it awards him a point for every card that extends the sequence. Examples:

Ace + Three + Five = 0 Points
Ace + Two + Three = 3 Points
Ace + Two + Three + Five = 3 Points
Ace + Two + Three + Four + Five + Six = 6 Points

Every kind of better solution is accepted, such as Linq and enumerable extensions.

\$\endgroup\$
  • 1
    \$\begingroup\$ What is your m_cards variable? What kind of game is this? \$\endgroup\$ – Simon Forsberg Dec 24 '13 at 0:28
7
\$\begingroup\$

This is a little nicer in my opinion. Here are the changes:

  • Start counting points from ace and reset to 0 if it's less than 3
  • Magic numbers named as constants
  • Primitive alias' used instead of the actual type keywords (int over Int32)
  • Put into a method
  • points declared ;)

private int GetSuitPoints(IEnumerable<Card> deck, Suit suit) {
    const int pointThreshold = 3;
    const int cardsPerSuit = 10;

    bool[] cards = new bool[cardsPerSuit];
    int i = 0;
    int points = 0;

    foreach (Card card in deck.Where(card => card.Suit == suit))
        cards[(int)card.Rank] = true;

    while (points == i) {
        if (cards[i++])
            points++;
    }

    if (points < pointThreshold)
        points = 0;

    return points;
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ +1 but you need to add in a return statement at the end return points; - also it might be good to rename the spades variable as you've made the method accept a suit argument. \$\endgroup\$ – RobH Mar 15 '13 at 12:55
  • \$\begingroup\$ +RobH cheers, fixed. \$\endgroup\$ – Daniel Imms Mar 15 '13 at 14:11
10
\$\begingroup\$

We can use the fact that in the ordered sequence of cards the cards that match your requirement will be located at the beginning of sequence, and their rank will match their position:

var count = _cards.Where(card => card.Suit == Suit.Spades)
    .Select(card => (int)card.Rank) //we only need the rank
    .Distinct() //ignoring multiple cards with the same rank
    .OrderBy(rank => rank) //sorting by rank
    .TakeWhile((rank, index) => rank == index) //taking all initial cards with index matching their rank
    .Count();

return count < 3 ? 0 : count;

Note that your code will perform better than this one (you'll notice that only if you run it thousands times a second) since you're using the fact that there are only 10 sequential values present in Rank.

| improve this answer | |
\$\endgroup\$

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.