Skip to content

Conversation

meltar
Copy link

@meltar meltar commented Feb 28, 2013

I completed the panda and tiger levels.

@meltar
Copy link
Author

meltar commented Mar 1, 2013

I'm planning to try the eagle part too.

blackjack.rb Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think this test is not necessary moving forward. You have this covered by the same inputs and call in it "should use the new format"

Testing both sides is good testing when you have an if statement. in this case, I think it's overkill

Also --- in general, should calls are a stronger test than should_not

Copy link
Author

Choose a reason for hiding this comment

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

I agree. I wanted to try it both ways since I'm new to testing, but the "should use new format" test covers everything for future testing. I will remove the other one.

@jwo
Copy link
Member

jwo commented Mar 1, 2013

Looks good! Some tips:

in your Card#to_s method, it got a bit complex. I like what you're doing -- you could move the logic into a "suit_name" class and call that. something like:

   def to_s
     [@value, suit_name].join("-")
   end

   def suit_name
     { :clubs => "C", :diamonds => "D", :spades => "S", :hearts => "H" }[@suit]
   end

Let me know what you think. your code was good -- just some other ideas on ways to structure.

@meltar
Copy link
Author

meltar commented Mar 2, 2013

I like that idea. I thought the suit_name should be separate in case something else needed to use it, but I wasn't sure were to put it.

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