HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Blackjack game with many conditionals and switches

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conditionalswithswitchesgamemanyandblackjack

Problem

I am a little new/rusty to this C++ stuff. Is this bad code? Can it be improved?

```
#include
#include
#include
#include
#include
bool _initialized_time = false;
void Wait() {
std::cout > _next_move;
switch(_next_move) {
case 'p':
Reset();
break;
case 'q':
_game_over = true;
break;
}
}
void UpdateLastCard(ePlayerType _type, card _card) {
int _count = 0;
_count = PlayerCardCount(_type) - 1;
if(_type == pPlayer) {
_player_cards[_count] = _card;
} else if (_type == pComputer) {
_computer_cards[_count] = _card;
}
}
void RewardWinner() {
if(PlayerHandValue(pPlayer) > PlayerHandValue(pComputer)) {
if(PlayerHandValue(pPlayer) >= 22) {
std::cout PlayerHandValue(pPlayer)) {
if(PlayerHandValue(pComputer) >= 22) {
std::cout > _next_move;
if(_next_move == 'q') {
_game_over = true;
}
} else if(_default_bet > _player_money) {
std::cout > _next_move;
if(_next_move == 'q') {
_game_over = true;
}
} else {
if(_player_stays == true && _computer_stays == true) {
RewardWinner();
if(_player_money == 0) {
std::cout > _next_move;
if(_next_move == 'q') {
_game_over = true;
}
} else if(_default_bet > _player_money) {
std::cout > _next_move;
if(_next_move == 'q') {
_game_over = true;
}
} else {
std::cout = 22) {
RewardWinner();
if(_player_money == 0) {
std::cout > _next_move;
if(_next_move == 'q') {
_game_over = true;
}
} else if(_default_bet > _player_money) {
std::cout > _next_move;
if(_next_move == 'q') {
_game_over = true;
}
} else {
std::cout > _next_move;
switch(_next_move) {
case 'd':
if(_deck_empty) {
ShuffleDeck();

Solution

Here are some suggestions for you:

bool _initialized_time = false;

int ReturnRandomNumber(int _start, int _end) {
  if(_initialized_time == false) {
    _initialized_time = true;
    srand(time(0));
  }
  int n = rand() % _end + _start;
  return n;
}


Avoid using global variables if at all possible. For this simple implementation of the game it probably does not matter, but unless you have a good reason to use them, it's just as well to get in the habit of avoiding them.

In this case you should just initialize the random number generator when the program starts:

int black_jack() {
  srand(time(0));  
  Initialize();
  ...


Also you should avoid using leading underscores in identifiers, see this answer for the reasoning. If needed you can use a trailing underscore instead, but don't add underscores for no reason.

About naming, drop the hungarian notation too.

Your Initializefunction does a lot more than initialize the game. It is also very big. You should consider breaking it up into several smaller functions, and give them names that describes what they do.

You do have a lot of good function with well chosen names already, like PromptNewGame, ShuffleDeck, GiveCard etc. Try to break the Initialize-function into similar chunks of well named functions.

The ReturnCardValue-function could probably be a lot smarter. Just handle the special cases, and return the card type for the rest. Something like this, perhaps:

int ReturnCardValue(const card & c) {
  if (c.type > Nine)
    return 10;
  else
    return reinterpret_cast(c.type);
}


This may require the layout of your card type enum to be slightly different. Notica also that I chose to pass the card struct by reference instead of by value (copy). For a small struct like this it's not a big deal, but for bigger structs it's nice to avoid the copying if it's not needed.

Also, you don't have to initialize every member of the enum as long as the values are consecutive. Try this instead:

enum CardType {
  Joker, AceSmall, Two, Three, Four, Five,
  Six, Seven, Eight, Nine, Ten, Jack, King,
  Queen, Ace
};


Finally you may want to look into turning several parts of your program into classes. From the top of my head I can think of at least three immediately obvious ones, Game, Player, and Card. This would encapsulate the functionality in each element of the game into logical units and make the program easier to read and maintain at the same time.

Allright, that's just some suggestions for you. Gives you at least somewhere to start.

Code Snippets

bool _initialized_time = false;

int ReturnRandomNumber(int _start, int _end) {
  if(_initialized_time == false) {
    _initialized_time = true;
    srand(time(0));
  }
  int n = rand() % _end + _start;
  return n;
}
int black_jack() {
  srand(time(0));  
  Initialize();
  ...
int ReturnCardValue(const card & c) {
  if (c.type > Nine)
    return 10;
  else
    return reinterpret_cast<int>(c.type);
}
enum CardType {
  Joker, AceSmall, Two, Three, Four, Five,
  Six, Seven, Eight, Nine, Ten, Jack, King,
  Queen, Ace
};

Context

StackExchange Code Review Q#23115, answer score: 3

Revisions (0)

No revisions yet.