Going through Head First C#'s Chapter 8: Enums and Collections, I learned about List<T>
, as well as IComparable<T>
and IComparer<T>
. One exercise near the end asked to make a program to draw cards at random, then sort them.
I wanted to go the extra mile and make a fully usable, realistic card deck (their solution just picked 5 values at random from 2 enums, which could result in drawing the same card twice).
Any and all advice is welcome, albeit please understand that I have yet to learn LINQ in a coming chapter, so if you suggest to use it for something please at least explain why/advantages over vanilla C#.
I'm not including the using
statement, they are the default ones that Visual Studio adds when creating a C# console project.
Kind
enum Kind
{
Ace,
Two,
Three,
Four,
Five,
Six,
Seven,
Eight,
Nine,
Ten,
Jack,
Queen,
King,
}
Suit
enum Suit
{
Clubs,
Diamonds,
Hearts,
Spades,
}
Card
class Card : IComparable<Card>
{
public Kind Kind;
public Suit Suit;
public Card(Kind kind, Suit suit)
{
Kind = kind;
Suit = suit;
}
public int CompareTo(Card other)
{
if (Suit > other.Suit)
{
return 1;
}
if (Suit < other.Suit)
{
return -1;
}
return Kind > other.Kind ? 1 : -1;
}
public override string ToString()
{
return $"{Kind} of {Suit}";
}
}
CardDeck
class CardDeck
{
public List<Card> Cards;
public CardDeck()
{
Cards = new List<Card>();
int numSuits = Enum.GetNames(typeof(Suit)).Length;
int numKinds = Enum.GetNames(typeof(Kind)).Length;
for (int suit = 0; suit < numSuits; suit++)
{
for (int kind = 0; kind < numKinds; kind++)
{
Cards.Add(new Card((Kind)kind, (Suit)suit));
}
}
}
public int CountCardsInDeck => Cards.Count;
public Card DrawTopCard()
{
Card drawnCard = Cards[0];
Cards.RemoveAt(0);
return drawnCard;
}
public Card DrawBottomCard()
{
int lastCardIndex = CountCardsInDeck - 1;
Card drawnCard = Cards[lastCardIndex];
Cards.RemoveAt(lastCardIndex);
return drawnCard;
}
public Card DrawRandomCard()
{
Random random = new Random();
int randomCardIndex = random.Next(CountCardsInDeck);
Card drawnCard = Cards[randomCardIndex];
Cards.RemoveAt(randomCardIndex);
return drawnCard;
}
public void AddCardOnTop(Card card)
{
if (!Cards.Contains(card))
{
Cards[0] = card;
return;
}
throw new InvalidOperationException($"Deck already contains card {card}.");
}
public void AddCardOnBottom(Card card)
{
if (!Cards.Contains(card))
{
Cards.Add(card);
return;
}
throw new InvalidOperationException($"Deck already contains card {card}.");
}
public void AddCardAtRandom(Card card)
{
if (!Cards.Contains(card))
{
Random random = new Random();
Cards[random.Next(CountCardsInDeck)] = card;
return;
}
throw new InvalidOperationException($"Deck already contains card {card}.");
}
public void Shuffle()
{
// Fisher-Yates shuffle method
Random random = new Random();
int n = CountCardsInDeck;
while (n > 1)
{
n--;
int k = random.Next(n + 1);
Card randomCard = Cards[k];
Cards[k] = Cards[n];
Cards[n] = randomCard;
}
}
public void Sort() => Cards.Sort();
public void Sort(IComparer<Card> comparer) => Cards.Sort(comparer);
public void WriteToConsole()
{
foreach (Card card in Cards)
{
Console.WriteLine(card);
}
}
}
CardOrderMethod
enum CardOrderMethod
{
SuitThenKind,
KindThenSuit,
}
CardSorter
class CardSorter : IComparer<Card>
{
public CardOrderMethod SortBy = CardOrderMethod.SuitThenKind;
public int Compare(Card x, Card y)
{
if (SortBy == CardOrderMethod.SuitThenKind)
{
if (x.Suit > y.Suit)
{
return 1;
}
if (x.Suit < y.Suit)
{
return -1;
}
return x.Kind > y.Kind ? 1 : -1;
}
if (SortBy == CardOrderMethod.KindThenSuit)
{
if (x.Kind > y.Kind)
{
return 1;
}
if (x.Kind < y.Kind)
{
return -1;
}
return x.Suit > y.Suit ? 1 : -1;
}
throw new NotImplementedException($"CardOrderMethod {SortBy} is not implemented.");
}
}
Program
class Program
{
static void Main(string[] args)
{
CardDeck cardDeck = new CardDeck();
cardDeck.Shuffle();
Console.WriteLine("---Shuffled deck---");
cardDeck.WriteToConsole();
CardSorter sorter = new CardSorter
{
SortBy = CardOrderMethod.SuitThenKind
};
cardDeck.Sort(sorter);
Console.WriteLine("---Sorted deck: SuitThenKind---");
cardDeck.WriteToConsole();
cardDeck.Shuffle();
sorter.SortBy = CardOrderMethod.KindThenSuit;
cardDeck.Sort(sorter);
Console.WriteLine("---Sorted deck: Kind Then Suit---");
cardDeck.WriteToConsole();
// Keep console open until a key is pressed
Console.ReadKey();
}
}
7 Answers 7
enum Kind
{
...
Queen,
King,
Joker // ???
}
Jokers is the joker. You're not considering jokers
class Card : IComparable<Card> { ...
I'm not convinced that there is a default comparison for cards? It could be misunderstood by consumers. On the other hand a deck of cards is always sorted by Suit
and then Kind
, but the comparison of cards is highly context dependent - dependent on the rules of the game. See further below.
public int CompareTo(Card other) { if (Suit > other.Suit) { return 1; } if (Suit < other.Suit) { return -1; } return Kind > other.Kind ? 1 : -1; }
can be simplified to:
public int CompareTo(Card other)
{
if (other == null) return 1;
if (Suit != other.Suit)
return Suit.CompareTo(other.Suit);
return Kind.CompareTo(other.Kind);
}
CardDeck
is maybe a little verbose. IMO Deck
is sufficient.
Cards = new List<Card>(); int numSuits = Enum.GetNames(typeof(Suit)).Length; int numKinds = Enum.GetNames(typeof(Kind)).Length; for (int suit = 0; suit < numSuits; suit++) { for (int kind = 0; kind < numKinds; kind++) { Cards.Add(new Card((Kind)kind, (Suit)suit)); } }
There is a simpler way to do this:
Cards = new List<Card>();
foreach (Suit suit in Enum.GetValues(typeof(Suit)))
{
foreach (Kind kind in Enum.GetValues(typeof(Kind)))
{
Cards.Add(new Card(kind, suit));
}
}
CountCardsInDeck
again: Count
is sufficient. What should it else count if not cards in the deck?
public Card DrawTopCard() { Card drawnCard = Cards[0]; Cards.RemoveAt(0); return drawnCard; } public Card DrawBottomCard() { int lastCardIndex = CountCardsInDeck - 1; Card drawnCard = Cards[lastCardIndex]; Cards.RemoveAt(lastCardIndex); return drawnCard; } public Card DrawRandomCard() { Random random = new Random(); int randomCardIndex = random.Next(CountCardsInDeck); Card drawnCard = Cards[randomCardIndex]; Cards.RemoveAt(randomCardIndex); return drawnCard; }
This can be simplified:
public Card DrawCardAt(int index)
{
if (index < 0 || index >= Count)
throw new ArgumentOutOfRangeException(nameof(index));
Card card = Cards[index];
Cards.RemoveAt(index);
return card;
}
public Card DrawTopCard()
{
return DrawCardAt(0);
}
public Card DrawBottomCard()
{
return DrawCardAt(Count - 1);
}
public Card DrawRandomCard()
{
Random random = new Random();
int index = random.Next(Count);
return DrawCardAt(index);
}
public void AddCardOnTop(Card card) { if (!Cards.Contains(card)) { Cards[0] = card; return; } throw new InvalidOperationException($"Deck already contains card {card}."); }
A cleaner way to make precautions:
public void AddCardOnTop(Card card)
{
if (Cards.Contains(card))
throw new InvalidOperationException($"Deck already contains card {card}.");
//Cards[0] = card;
Cards.Insert(0, card);
}
You replace the existing first card with a new one. Is that what you want? If so the method should be called SetTop()
or ReplaceTop()
. My suggestion is that you want to insert? The same could be said about AddCardOnBottom()
and AddCardAtRandom()
.
public void WriteToConsole() { foreach (Card card in Cards) { Console.WriteLine(card); } }
Keep the UI out of your models. You could override ToString()
and produce a line-string instead.
class CardSorter
strictly speaking it is not a sorter but a comparer.
As discussed above, I maybe think that your comparer is somewhat redundant, because the comparison of cards depends on the game rules. I'm not a huge fan of card games, but I can't image games or situations where a deck of cards should be sorted differently than just by Suit
and then Kind
. But if you insists, you should consider other sorttypes:
enum CardSortType
{
KindOnly, // HH: In some games only the kind matters
SuitOnly, // HH: I can't image any games where this is used??
SuitThenKind,
KindThenSuit,
}
class CardSorter : IComparer<Card>
{
public CardSorter(CardSortType sortBy = CardSortType.SuitThenKind)
{
SortBy = sortBy;
}
public CardSortType SortBy { get; } // HH: Make it readonly
public int Compare(Card x, Card y)
{
switch (SortBy)
{
case CardSortType.KindOnly:
return x.Kind.CompareTo(y.Kind);
case CardSortType.SuitOnly:
return x.Suit.CompareTo(y.Suit);
case CardSortType.SuitThenKind:
if (x.Suit != y.Suit) return x.Suit.CompareTo(y.Suit);
return x.Kind.CompareTo(y.Kind);
case CardSortType.KindThenSuit:
if (x.Kind != y.Kind) return x.Kind.CompareTo(y.Kind);
return x.Suit.CompareTo(y.Suit);
default:
throw new NotImplementedException($"CardOrderMethod {SortBy} is not implemented.");
}
}
}
In the above, I suggest a simpler comparison.
-
1\$\begingroup\$ Excellent review. Is a joker a
Kind
though? Could you have the Joker of Clubs? Or the Joker of Diamonds? I would either put it as aSuit
(and preferably setKind
to null when used) or keep it as a separate property from both Kind and Suit. \$\endgroup\$Simon Forsberg– Simon Forsberg2019年07月06日 11:55:42 +00:00Commented Jul 6, 2019 at 11:55 -
\$\begingroup\$ @SimonForsberg: You're probably right. I rarely play cards so I'm not the one to consult. But introducing jokers will give you some extra work anyway. I think, I maybe would have an
abstract class Card
and then two derived:class NormalCard : Card
andclass Joker : Card
, but I haven't thought that through... \$\endgroup\$user73941– user739412019年07月06日 12:01:21 +00:00Commented Jul 6, 2019 at 12:01 -
\$\begingroup\$ Personally I feel that abstract classes are often very overused (but maybe that's just in Enterprise), it is a tricky issue to solve that can be solved in many different ways. Again, excellent review :) \$\endgroup\$Simon Forsberg– Simon Forsberg2019年07月06日 12:04:36 +00:00Commented Jul 6, 2019 at 12:04
-
1\$\begingroup\$ There're plenty of games with "weird" sorting rules. For example Canasta (en.wikipedia.org/wiki/Canasta) ignores suits alltogether pretty much and only cares for kinds. Russian Schnapsen (en.wikipedia.org/wiki/Russian_Schnapsen) has weird kind order (A 10 K Q W 9) and different order of suits (hearts diamonds clubs spades if im not mistaken). Rummy (en.wikipedia.org/wiki/Rummy) probably should sort suits, but prefers the same kind together. And so on. \$\endgroup\$Radosław Cybulski– Radosław Cybulski2019年07月06日 15:57:29 +00:00Commented Jul 6, 2019 at 15:57
-
2\$\begingroup\$ Yes, you're right. It depends, what is sorting used for. Is it for drawing hand? Sorting cards by their "strength"?. I just mentioned, that in card games there's plenty of "sensible" sorting, depending on the game itself. \$\endgroup\$Radosław Cybulski– Radosław Cybulski2019年07月06日 16:27:21 +00:00Commented Jul 6, 2019 at 16:27
Just a few things not already mentioned by other answers:
Are you sure that you will need DrawRandomCard()
? How many card games have you played where you just draw from a random position in the deck? The only times I can think of this is useful is when you do a magic trick, which doesn't really work the same in code as in real life :) Normally in card games, you shuffle the deck and then draw from top or bottom.
Different card games might want to sort the deck in different ways, as others have mentioned already, but there's more options as well. Does Aces count as high or low? (Or both?) Should the sort order of the suits be [Clubs, Diamonds, Hearts, Spades], or [Clubs, Diamonds, Spades, Hearts], or [Diamonds, Hearts, Spades, Clubs] ? This can be dependent on the type of game, and maybe also a player's preferences. Adding more flexible options for Ace low/high and Suit order to your comparer would be good.
List<T>
You have just learned about List<T>
and are eager to use it. Perfectly understandable, but think about what it is designed for and what alternatives are available for this use case. Here is an overview of common Collections in C#
To be fair, there is no common collection available designed for a deck of cards. What you need, is functionality from:
HashSet<T>
- unique items in the deckQueue<T>
- add card on one end, take card from the other endStack<T>
- add card on one end, take card from that end
You have opted to use List<T>
and added the above functionality yourself. This is not a bad solution, but you should realise it is not optimized for a deck of unique cards.
Card
You can compare enum values the same way you are comparing instances of Card
.
public int CompareTo(Card other) { if (Suit > other.Suit) { return 1; } if (Suit < other.Suit) { return -1; } return Kind > other.Kind ? 1 : -1; }
The above can be written as:
public int CompareTo(Card other)
{
if (other == null) return -1;
var index = Suit.CompareTo(other.Suit);
if (index == 0) index = Kind .CompareTo(other.Kind);
return index;
}
Since Card
is used by CardDeck
and the latter uses a List<Card>
with unique card items, you should do at least one of either and preferrably both:
- override
Equals()
andGetHashCode()
- implement
IEquatable<Card>
This is because List<T>
uses EqualityComparer<T>
to check for unique instances (Reference Source).
code snippet (only equality snippets):
class Card : IEquatable<Card>
{
public void Equals(Card card)
{
if (card == null) return false;
return card.Suit == Suit && card.Kind == kind;
}
public override void Equals(object obj)
{
if (!(obj is Card card)) return false;
return card.Suit == Suit && card.Kind == kind;
}
public override int GetHashCode()
{
// prefer picking two numbers that are co-prime
var hash = 23;
hash = hash * 31 + Suit.GetHashCode();
hash = hash * 31 + Kind.GetHashCode();
}
}
CardDeck
In your next lesson you will learn about LINQ. You will be able to write the card generator function as follows:
foreach (var card in (from suit in Enum.GetValues(typeof(Suit)).Cast<Suit>()
from kind in Enum.GetValues(typeof(Kind)).Cast<Kind>()
select new { suit, kind }))
{
Cards.Add(new Card(card.kind, card.suit));
}
as compared to:
int numSuits = Enum.GetNames(typeof(Suit)).Length; int numKinds = Enum.GetNames(typeof(Kind)).Length; for (int suit = 0; suit < numSuits; suit++) { for (int kind = 0; kind < numKinds; kind++) { Cards.Add(new Card((Kind)kind, (Suit)suit)); } }
For shuffling and drawing random cards, consider declaring the Random
as a private variable of the deck rather than inside the methods. This avoids the not so random Random behavior.
private readonly Random random = new Random();
Consider inverting code like this:
if (!Cards.Contains(card)) { Cards[0] = card; return; } throw new InvalidOperationException($"Deck already contains card {card}.");
to this:
if (Cards.Contains(card))
{
throw new InvalidOperationException($"Deck already contains card {card}.");
}
Cards[0] = card;
Don't pollute your classes with specific utility methods. Write this as a static method or extension method in your test code.
public void WriteToConsole() { foreach (Card card in Cards) { Console.WriteLine(card); } }
CardSorter
You should prefer CompareTo
over comparasion operators.
if (x.Suit > y.Suit) { return 1; } if (x.Suit < y.Suit) { return -1; } return x.Kind > y.Kind ? 1 : -1;
var index = x.Suit.CompareTo(y.Suit);
if (index == 0) index = x.Kind.CompareTo(y.Kind);
return index;
Also include null comparisons.
if (x == null) return (y == null) ? 0 : 1;
if (y == null) return -1;
General guidelines
- use
var
as much as you can, specially when the declared type can be inferred from reading the code.var card = new Card();
reads better asCard card = new Card();
orDictionary<string, List<int>> collection = new Dictionary<string, List<int>>();
vsvar collection = new Dictionary<string, List<int>>();
- check arguments against
null
in public methods
-
\$\begingroup\$ Can you explain the reason to use var as much as possible? \$\endgroup\$user161670– user1616702019年07月06日 09:15:18 +00:00Commented Jul 6, 2019 at 9:15
-
\$\begingroup\$ @BlackBox edited an explanation \$\endgroup\$dfhwze– dfhwze2019年07月06日 09:19:25 +00:00Commented Jul 6, 2019 at 9:19
Encapsulation and Mutability
Your Card
class is a perfect candidate for being immutable: make Kind
and Suit
readonly fields or getter-only properties (prefer the later in most cases):
public Kind Kind { get; }
public Suit Suit { get; }
You should also hide away the Deck
list in CardDeck
: making it readonly and private
(or protected
, if you are happy to commit to (and document) the internal representation) would probably be most sensible. Prevent the user shooting themselves in the foot by messing with state they shouldn't be able to access.
Card..ctor(Kind, Suit)
Enums
in .NET are very simple, and you have no gaurentee that a Kind
is actually one of those you have declared. I would consider adding checks here to ensure that an invalid Kind
or Suit
is not used: catch the mistake early, because it will go wrong somewhere down the line, and the sooner you throw an exception at the consumer, the easier it will be for them to work out where they went wrong.
Comparers
Dfhwze's Answer indirectly addresses this, but your Card.CompareTo(Card)
can never return 0
: this is very bad. It should check if the cards are equivalent, and return 0
in that instance and that instance only.
As alluded by Henrik Hansen, I would rename CardSorter
to CardComparer
: it's what everyone will assume, and I would certainly expect a CardSorter
to provide a 'Sort' method. Again, the comparisons here don't allow for the same card appearing twice: your code CardDeck
class may assume they are never equal, but other people may try to use this class for other purposes. These sorts of things need to be documented.
Misc
Why should adding an already present card disallowed? Some cards games involve multiple complete 'decks' in a single 'deck'.
This all needs documention. As a consumer, I need to know that
AddCardOnBottom
won't accept an already present card.As discussed by Henrik,
CardDeck.Sort
is a bit odd: I would remove it, and force the user to specify the type of sort they want (by using theCardSorter
class).I would rename
CardSorter
toCardComparer
: it's what everyone will assume, and I would certainly expect aCardSorter
to provide a 'Sort' method. Again, the ccomparisons here don't allow for the same card appearing twice: your code may not allow this, but other people may try to use this class for other purposes.To reiterative dfhwze 's point, you should not be creating a new
Random
instance inShuffle
: either encapsulate one in the class which you take as a parameter to the constructor, or allow theShuffle
and other 'Random' methods to take one as a parameter.You might consider making the type of your
enum
s explicit (e.g. both fit in abyte
), and you could make yourCard
class an immutable struct with the same semantics, only it would take up less space and reduce the GC overhead.
-
\$\begingroup\$ Good catch about the immutability. You could even make the
Deck
"immutable", so it maintains a set of unused and used cards, which will prevent cheating :-) \$\endgroup\$user73941– user739412019年07月06日 12:13:52 +00:00Commented Jul 6, 2019 at 12:13
First off, this is quite good for beginner code. You'll do well if you keep the good habits you've started following.
A few critiques:
class Card : IComparable<Card>
Do you intend this class to be subclassed? If not, seal it.
Are cards comparable by reference? That is, suppose we have two different instances of the queen of spades. Is it possible for the user of the library to tell them apart, or not? If no, then this should be a struct, not a class; force the user to compare by value.
public Kind Kind;
public Suit Suit;
NEVER DO THIS. Never make a public field. First, because it is a bad practice, and second, because now anyone can have a card in hand and change it. Cards are immutable! This should be
public Kind Kind { get; private set; }
And so on.
public int CompareTo(Card other)
This code is very wrong. x.CompareTo(x)
must always return zero, always. You never tested that case, otherwise you would have found the bug, so write that test.
If you're going to implement CompareTo
it would be a good practice to also implement Equals
, operator ==
, operator !=
, GetHashCode
, operator >
, operator <
, operator <=
and operator >=
. It is weird that you can compare two cards for rank with CompareTo
but not with <
or ==
.
class CardDeck
Again, seal it.
public List<Card> Cards;
Again, never make a public field. This should be private; it's an implementation detail of the class.
public int CountCardsInDeck => Cards.Count;
Needlessly verbose; just public int Count => Cards.Count
is fine.
public Card DrawTopCard()
Throw a better exception if there is no top card.
Random random = new Random();
Newer versions of .NET have fixed this, but in older versions this was a bad practice, creating a Random every time you needed it. Instead, make it a field.
public void WriteToConsole()
This is a weird way to write this functionality. Normally you'd override ToString
and then do Console.WriteLine(deck);
class CardSorter : IComparer<Card>
{
public CardOrderMethod SortBy = CardOrderMethod.SuitThenKind;
Again, seal your classes, and again, no public fields.
But this design is wrong. Don't make one class that can do two things. Make two classes if you have two things to do. Make a SuitThenKindSorter
and a KindThenSuitSorter
class, not one class that has an if
in the middle.
And again, the comparison logic is wrong; you are required to have a comparison where things that are equal are equal. You must never assume that the things being compared are unequal. The contract of the comparison is that it can take any two objects of the type and compare them, not any two different objects.
-
\$\begingroup\$ seal it - I read your article about it here: Why Are So Many Of The Framework Classes Sealed? and I find that the advantages of
sealed
classes are overrated and thier advantages highly questionable and you have to break them with reflection to get the job done because someone thought they knew better than anyone else how a particular class should and would be used. \$\endgroup\$t3chb0t– t3chb0t2019年07月08日 06:02:16 +00:00Commented Jul 8, 2019 at 6:02 -
4\$\begingroup\$ @t3chb0t: Do you feel the same way about non-virtual methods? Every non-virtual method is the developer "thinking they know better about how a method should be used". What about private methods? For that matter, why have methods at all? Just make all methods into fields of delegate type so that any developer can change the behavior of any method without overloading. Why have private members? Private members are developers foolishly believing they know how their classes will be used, right? Where does your argument stop, and why? \$\endgroup\$Eric Lippert– Eric Lippert2019年07月08日 14:15:38 +00:00Commented Jul 8, 2019 at 14:15
-
1\$\begingroup\$ non-sealed classes are more useful because they allow you to create nice aliases for built-in types like for a dictionary or a list. I'm very happy that I can have a oneliner class like
class UserNameDictionary : Dictionary<int, string> {}
that I can easily reuse everywhere without having to writeusing
s which aStringBuilder
disallows so I cannot create aUserNameBuilder : StringBuilder {}
. non-virtual method can be forcibly overriden withnew
which I think you have added because of the lack of virtual by default like java does that ;-) and there is nounseal
spell. \$\endgroup\$t3chb0t– t3chb0t2019年07月08日 14:31:01 +00:00Commented Jul 8, 2019 at 14:31 -
4\$\begingroup\$ @t3chb0t The idea of
class UserNameDictionary : Dictionary<int, string> {}
goes against the concept of "prefer composition over inheritance". I bet that when you make such a class, you won't be using all of the inherited methods. It's also a lot trickier to change the implementation of it later if you extend from another class. \$\endgroup\$Simon Forsberg– Simon Forsberg2019年07月09日 21:27:13 +00:00Commented Jul 9, 2019 at 21:27 -
4\$\begingroup\$ @SimonForsberg: I completely agree, but I would make an even more general argument. The position taken here is "unseal your classes because C# has a crappy mechanism for aliases". But this is basically the same position as "use the same password on every site because humans have crappy memory for multiple passwords". If "crappy mechanism for aliases" is the problem then advocate for fixing that problem. Don't encourage everyone else to use bad practices to work around it. \$\endgroup\$Eric Lippert– Eric Lippert2019年07月09日 21:32:11 +00:00Commented Jul 9, 2019 at 21:32
Sorry, I pressed enter on accident, instead of getting AC, we get the index instead which is one I believe. It is possible to get AC to KS (Ace of Clubs to King of Spades) with the use of enums but in my opinion, getting the index is better.
The method displays how it will choose a random card from the list, which can be used wherever it is needed.
Hopefully this clears up the benefit of this method
-
3\$\begingroup\$
I pressed enter on accident
trying to do what? Is this an amendment to your 2023年03月22日 answer? Use Edit there. \$\endgroup\$greybeard– greybeard2023年06月05日 07:56:31 +00:00Commented Jun 5, 2023 at 7:56
public void Shuffle()
{
for (int i = cards.Count - 1; i > 1; i--)
{
int pos = rnd.Next(i);
PlayingCard tmp = cards[i];
cards[i] = cards[pos];
cards[pos] = tmp;
}
}
This is a basic shuffle method that u can call in your forms somewhere. Making changes in the for loop where 1 in the condition is a request from user could work.
-
1\$\begingroup\$ Welcome to Code Review! Are you suggesting the OP use this code instead of the code inside the ‘Shuffle()` method on the
CardDeck
class? If so, please edit the answer to explain what benefits this would offer. \$\endgroup\$2023年03月22日 12:13:40 +00:00Commented Mar 22, 2023 at 12:13 -
\$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$Toby Speight– Toby Speight2023年03月22日 12:58:56 +00:00Commented Mar 22, 2023 at 12:58
Explore related questions
See similar questions with these tags.
Ace
is ranked belowTwo
in the enum? Most games rank theAce
as the highest card. \$\endgroup\$