Skip to content

Pr#1

Open
GiulianoPiz wants to merge 5 commits intodivega:masterfrom
GiulianoPiz:PR
Open

Pr#1
GiulianoPiz wants to merge 5 commits intodivega:masterfrom
GiulianoPiz:PR

Conversation

@GiulianoPiz
Copy link

No description provided.


namespace TechnicalAssesment
{
public enum GameResult
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Enums to its own file. Away from the game logic. EG: Enums/GameResult.cs


public class ScoreBoard
{
public byte playerWins;
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use int type, there will be limitations on larger values being stored in bytes.
Also there are inconsistencies in first letter casing. EG: public byte playerWins --> public byte PlayerWins

{
public byte playerWins;
public int DealerWins;
public byte Ties { get; set; }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use int type, there will be limitations on larger values beeing stored in bytes.

Comment on lines +30 to +36
public void Shuffle()
{
for (int i = 0; i < MaxCards; i++)
{
cards.Add(i);
}
}
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shuffle method does not cater for any card type Eg: Hearts, Clubs etc.
The value of the cards will not represent the face value of the card Eg: King = 13, Queen = 12 etc.

public byte Ties { get; set; }
}

public class Deck
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Deck to its own file. Under services. EG EG: Services/DeckService.cs


internal void Play(int numGames)
{
deck = new Deck();
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can new up the Deck class in the game class on construction.

internal void Play(int numGames)
{
deck = new Deck();
ShuffleDevk();
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling error. This may not compile/build.

Comment on lines +88 to +102
case GameResult.DealerWon:
ScoreBoard.DealerWins++;
Console.WriteLine($"{numGames}: Dealer");

break;

case GameResult.PlayerWon:
ScoreBoard.playerWins++;
Console.WriteLine($"{numGames}: Player");
break;

case GameResult.Tie:
ScoreBoard.Ties++;
Console.WriteLine($"{numGames}: Tie");
break;
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The console output does not show the results of any drawn card value, only the number of games/rounds played.


}

private void ShuffleDeck()
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No use for "ShuffleDeck" method in game class simply call the deck class from the main game.

Comment on lines +69 to +70
private byte DealerCard;
private byte PlayerCard;
Copy link
Author

@GiulianoPiz GiulianoPiz Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to for class declaration as the values are changed each time a draw takes place and can be a local variable within the method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant