Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BlackJack - E1 #33

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

BlackJack - E1 #33

wants to merge 3 commits into from

Conversation

var114
Copy link

@var114 var114 commented Sep 10, 2013

No description provided.

@var114
Copy link
Author

var114 commented Sep 10, 2013

E1

(2..10).each do |number|
cards << Card.new(suit, number)
end
["J", "Q", "Q", "A"].each do |facecard|
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