Skip to content

Conversation

@var114
Copy link

@var114 var114 commented Sep 10, 2013

No description provided.

@var114
Copy link
Author

var114 commented Sep 10, 2013

E1

Copy link
Member

Choose a reason for hiding this comment

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

Probably a typo, but you wanted ["J", "Q", "K", "A"] (you have 2 queens)

@jwo
Copy link
Member

jwo commented Sep 11, 2013

Hi!

When I run your specs, I get

Pending:
  Game should only hit when player hand is not bust
    # Not yet implemented
    # ./blackjack.rb:226

I'd remove this since it looks like it's not complete.

Your code looks fine, short methods and good variable names. I recommend you look at two fairly important things for readability of your code:

  1. your spacing: it should be two spaces indented. I pointed out a couple of places this wasn't followed (but there were more). It may seem like a small thing, but when scanning code, it's the inconsistencies that slow you down.
  2. make sure you don't leave old code or comments in the code. later-you won't remember why.

finally:

  def hit    #Tig1.2
    if player_hand.value > 21
      stand
    elsif player_hand.value < 21  
      @player_hand.hit!(@deck)
      standfor 
    elsif player_hand.value == 21
      stand
    end
  end

This has no else. I recommend adding one there, even if its raise "illegal player_hand #{player_hand.value}" Also, since you have stand there twice, you could:

if player_hand.value == 21 || player_hand.value > 21
  stand
else
  @player_hand.hit!(@deck)
   standfor 
end

However, that could even be reduced to:

if player_hand.value >= 21
  stand
else
  @player_hand.hit!(@deck)
   standfor 
end

finally, in the if/else block, you are using player_hand in one, and @player_hand in the other. You should be consistent.

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