Deck of cards

[Suzie999]

Although I've been programming as a hobby for a while, most of it has been procedural and I've done very little OOP, and not very competent with the inheritance hierarchy of classes, which is the reason for my post here, to get a bit of advice as to whether im thinking correctly.

As a long term project I've decided to make a game of texas holdem poker and get more familiar with OOP as I go along.

First port of call is to create a class for a deck of cards, and my first question is whether a deck of cards should have a base class of card like so..

namespace CS_TexasHoldem
{
    class Card
    {
        private char suit { get; set; }
        private char value { get; set; }

        public Card()
        {

        }
    }

    class Deck : Card
    {
        private ushort deck_count { get; set; }

        public Deck()
        {
            deck_count = 52;
        }
    }
}

Of course I've only just begun, but I don't want to get too far in and realise I'm wrong.

I'm aware that there are probably ready made card classes around, but I'd not learn much from using one of them, and I'd sooner not even look at one yet (just my way of learning)

Am I on the right track? is the basic question I suppose.

[deceptikon]

my first question is whether a deck of cards should have a base class of card

It doesn't make sense to me, personally. A deck of cards is not a card, it's a collection of cards, so composition is the better choice over inheritance:

public class Deck
{
    public List<Card> Cards { get; set; }

    ...
}

[Suzie999]

That makes sense.

I was beginning to wonder as I was Thinking about how to constuct the Deck class.

Thank you for your time.

[Suzie999]

Is there a point of having private fields like so...

private char suit { get; set; }
private char value { get; set; }

I cannot get or set them from an instance of the class.

Do they have to be public or is there something I'm missing?

[darkagn]

Either set them to public or pass them in via the constructor. A third option is to have get public but set private. The syntax for this option is as follows:

public char suit { get; private set; }
public char value { get; private set; }

public Card(char aSuit, char aValue)
{
    suit = aSuit;
    value = aValue;
}

This option means that the card's suit and value can be accessed in a read-only manner from outside the instance.

[Suzie999]

Here is my Card class so far.
If anyone has the time to look it over I'd be grateful.

I have one issue which I will need to address, and that is comapring an emum member (if that is what they're called) to my UInt16 (I have tried ushort too).

It appears that you cannot compare a value from an enum to even an integral type, and as you will see in below code (line 57) I have to first convert it, so any advice on a better way to do that would be great.

Thanks for taking the time to look.

namespace CS_TexasHoldem
{
    public enum CARDVALUE : ushort
    {
        TWO = 2,
        THREE,
        FOUR,
        FIVE,
        SIX,
        SEVEN,
        EIGHT,
        NINE,
        TEN,
        JACK,
        QUEEN,
        KING,
        ACE
    }

    public enum CARDSUIT
    {
        /*using ascii values*/
        CLUBS = 99,
        DIAMONDS = 100,
        HEARTS = 104,
        SPADES = 115
    }

    class Card
    {

        private UInt16 psuit = 120;
        private UInt16 pvalue = 88;

        public UInt16 suit
        {
            get { return psuit; }
            set { psuit = value; }
        }

        public UInt16 value
        {
            get { return pvalue; }
            set { pvalue = value; }
        }

        public Card(UInt16 avalue, UInt16 asuit)
        {
            /*Test that arguments ar within bounds*/
            Debug.Assert((avalue > 1 && avalue < 15) && (asuit == 99 || asuit == 100 || asuit == 104 || asuit == 115));
            psuit = asuit;
            pvalue = avalue;
        }

        public override String ToString()
        {
            if (pvalue < Convert.ToUInt16(CARDVALUE.TEN))
            {
                return pvalue.ToString() + ((char)psuit).ToString();
            }

            switch (pvalue)
            {
                case 10:
                    return "T" + ((char)psuit).ToString();
                case 11:
                    return "J" + ((char)psuit).ToString();
                case 12:
                    return "Q" + ((char)psuit).ToString();
                case 13:
                    return "K" + ((char)psuit).ToString();
                case 14:
                    return "A" + ((char)psuit).ToString();
                default:
                    return "Xx";
            }
        }
    }

    class Deck
    {
        // TODO
    }
}

[ddanbe]

The whole purpose of an enum is to avoid doing things like you do on line 49. asuit can only have the values CARDSUIT.CLUBS, CARDSUIT.DIAMONDS and so on. You cannot pass 99 to your method and expect it to be clubs. I guess that works in C++, C# is much more rigourous about that.
EDIT: in fact that's why I like C# so much!

[deceptikon]

The problem seems to be in your ToString override in that you want a format that's difficult to attain using enumerations. It can be done, of course. I'd probably approach it something like this:

namespace CS_TexasHoldem
{
    public enum CardSuit : int
    {
        CLUBS = 'c',
        DIAMONDS = 'd',
        HEARTS = 'h',
        SPADES = 's'
    }

    public enum CardValue : int
    {
        TWO = 2,
        THREE,
        FOUR,
        FIVE,
        SIX,
        SEVEN,
        EIGHT,
        NINE,
        TEN,
        JACK = 'J',
        QUEEN = 'Q',
        KING = 'K',
        ACE = 'A'
    }

    class Card
    {
        private static Dictionary<CardSuit, string> _cardSuits;
        private static Dictionary<CardValue, string> _cardValues;

        static Card()
        {
            _cardSuits = new Dictionary<CardSuit, string>();
            _cardValues = new Dictionary<CardValue, string>();

            foreach (var suit in (CardSuit[])Enum.GetValues(typeof(CardSuit)))
            {
                _cardSuits.Add(suit, ((char)suit).ToString());
            }

            foreach (var value in (CardValue[])Enum.GetValues(typeof(CardValue)))
            {
                if ((int)value <= 10)
                {
                    _cardValues.Add(value, ((int)value).ToString());
                }
                else
                {
                    _cardValues.Add(value, ((char)value).ToString());
                }
            }
        }

        public CardSuit Suit { get; private set; }
        public CardValue Value { get; private set; }

        public Card(CardSuit suit, CardValue value)
        {
            Suit = suit;
            Value = value;
        }

        public override String ToString()
        {
            return _cardSuits[Suit] + _cardValues[Value];
        }
    }
}

Note that the public interface only supports the enumerations. This saves you trouble checking against invalid values. The class then manages conversion of those enums into a friendly string format.

However, a better approach would be to avoid that string conversion entirely and let the calling application decide how it wants to handle display of cards. Right now you're doing something low level that's probably not necessary given that calling applications will likely want to display images in a GUI rather than text in a console.

[Suzie999]

Thank you both for the direction.
Will take it all in.