Code Review - Harry

[Harry1308]

Pros

• Well-structured: Functions and methods are divided into logical functional blocks, and the code is set up in a concise and easily understood manner.

• Clear and informative variable and function names make it simple to understand what each component of the code does.

• Style: The code is written in a consistent manner that makes it simple to read and comprehend.

Improvements

• Given that testPack is static, the setup() function should likewise be static. The warning that "static member... should be accessible through type name" would not apply in this case.

• FormatCard() and FormatCards() can be simplified with string interpolation:

private static string FormatCard(Card card)
{
    return $"{card.Suit}-{card.Value}";
}

private static string FormatCards(List<Card> cards)
{
    return string.Join("", cards.Select(card => FormatCard(card)));
}

• In order to prevent other tests from depending on tests that affect the state of the pack, such as NoShuffleTest(), they should have a separate setup() function.

• DealRiffleTest(), DealCardRiffleShuffle(), and DealNoShuffleTest() all rely on the pack's order after shuffling, which might be altered if the shuffle algorithm is changed. Before doing the tests, it would be better to generate a known pack order and shuffle it using each algorithm.

• The intended string is cut short at the end of FischerYatesShuffleTest() because it is too lengthy. Use of a string builder rather than a string literal or the use of a smaller pack for testing might both help to address this.

• There are no tests for edge cases like dealing from an empty pack or dealing more cards than there are in the pack. To increase the test suite's scope, these could potentially be added.

Overall, it seems like the code was well-written, and the tests cover what the pack of cards is supposed to do.

[Day6669]

I didn't think of this! Thankyou!