Skip to content

mnaser submission#2

Open
Git-Leon wants to merge 12 commits intocurriculeon:masterfrom
mnaser11218:master
Open

mnaser submission#2
Git-Leon wants to merge 12 commits intocurriculeon:masterfrom
mnaser11218:master

Conversation

@Git-Leon
Copy link
Contributor

No description provided.

}
}

hit() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not make sense to say

A BlackJackGameState can hit.

It makes sense to say

A BlackJackPlayer can hit.

Never compromise design for functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const playerScore = this.blackJackGameData.getPlayer().getHandTotal();
const dealerScore = this.blackJackGameData.getDealer()
const playerScore = this.blackJackGameData.getPlayer()
if (playerScore > dealerScore && playerScore < 22) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if playerScore is a Player object, how could the line below be a reasonable expression?
playerScore < 22.

playerScore is not an integer, so how could we compare it to 22?

}


create(player) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it makes more sense to me for communication to backend to be handled in a separate class and file.
Perhaps consider creating a BlackJackGameService.

// a blackjack player should receive a name when created
// a black jack player's hand is empty until receiving cards from a dealer
constructor(name) {
constructor(name, cards) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

passing cards as an argument seems a bit useless.
This is especially true when we consider that the argument is never used, but even if it were used, I can't imagine it being useful.

@JoinColumn(name = "player_id")
private List<Card> cards;

public Player() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should rename this class to BlackJackPlayerState.


@RequestMapping(value="/read-all", method= RequestMethod.GET)
public ResponseEntity<List<Player>>readAll(){
return null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no method implementation here?
this should have CRUD operations at the least

this.blackJackGameDataView.setNumberOfCardsOnScreen();
this.blackJackGameDataView.checkAndUpdateWinner();
this.blackJackGameDataView.setActiveOnCurrentPlayer();
if( !this.blackJackGameData.isCurrentPlayerDealer()||(this.blackJackGameData.isCurrentPlayerDealer() && this.blackJackGameData.getCurrentPlayer().getHandTotal() <=16)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems like this logic should be moved to the BlackJackDealer and BlackJackPlayer class

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.

2 participants