joncodo / Crib

A crib engine to play the card game crib with AI and a card counter
3 stars 2 forks source link

deck_spec.rb: Code Review #9

Closed scott-steadman closed 10 years ago

scott-steadman commented 10 years ago

Individual recommendations in comments on this ticket.

scott-steadman commented 10 years ago

I suggest wrapping the both describes in a `describe Deck do block and getting rid of the Deck parameter in the current describe blocks.

scott-steadman commented 10 years ago

Refactor the before(:each) into a deck method. Reason: DRY up your code and lazily create Decks.

Example code:

def deck
  @deck ||= Deck.new
end
scott-steadman commented 10 years ago

should_deal_a_card should make sure a Card was returned. Reason: testing for nil will be false if a non-Card is returned as well...

it 'should deal a card' do
    expect(@deck.deal_one.class).to eq(Card)
end
scott-steadman commented 10 years ago

Use .to be_false rather than .to eq(false) Unless you are specifically testing for false as opposed to nil. Reason: This looks like how rspec intended it to be used.

joncodo commented 10 years ago

Awesome. This is my first RSpec project so I'm just getting used to it