Episode 1 Assignments #10

Open
wants to merge 4 commits into
from

Projects

None yet

2 participants

@Meshed

I even threw in an extra.

@jwo jwo commented on an outdated diff Nov 16, 2012
blackjack.rb
@@ -14,7 +14,11 @@ def value
end
def to_s
- "#{@value}-#{suit}"
+ suit = "C" if @suit == :clubs
@jwo
jwo Nov 16, 2012

I think this could be confusing to later-you in a couple of months... There''s an attr_reader :suit in this class. You're not writing over that variable (since it was a reader), but you could be if that gets changed later on.

Possibly something like this would be more explicit

def to_s
  "#{suit_abbreviation}-#{value}"
end

def suit_abbreviation
  case suit
  when :diamonds then "D"
  when :hearts then "H"
  when :spaids then "S"
  when :clubs then "C" 
  end
end

Of course, you could even do this if you wanted

def to_s
  "#{suit.to_s[0].upcase}-{value}"
end
@jwo jwo commented on an outdated diff Nov 16, 2012
blackjack.rb
@@ -83,10 +88,17 @@ def stand
end
def status
+ if @winner == nil
@jwo
jwo Nov 16, 2012

better to say

if @winner.nil?
@jwo
Ruby Off Rails member

well done! I left some stylistic suggestions... let me know what you think!

@jwo
Ruby Off Rails member

OK, so this failed the build with TravisCI (https://travis-ci.org/RubyoffRails/Episode1-Summer2012/builds/3411231)

Why do you think it's failing?

@Meshed
@jwo
Ruby Off Rails member

Yep yep --- I wish we could have variable names with dashes in Ruby, but sadly negative.

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