card.rb: Code Review

[scott-steadman]

Each suggestion is in a comment below...

[scott-steadman]

Change number to rank Reason: Improve code readability by using terms from the domain.

Cards have a rank (A, 2, 3...) and a suit (Hearts, Clubs, Spades, Diamonds).

[scott-steadman]

Refactor display_value to to_s. Reason: to_s is ruby's idiomatic way of representing a class as a string.

Get rid of the display_value field, and add a to_s method that does the same thing.

def to_s
  "#{rank}-#{suit}"
end

[scott-steadman]

Verify parameters in constructor Reason: This will prevent invalid cards (eg 27 of FooBars)

Example code:

class Card

  def initialize(rank, suit)
    raise ArgumentError 'Invalid Rank' unless Card.ranks.include?(rank)
    raise ArgumentError 'Invalid Suit' unless Card.suits.include?(suit)
    self.rank = rank
    self.suit = suit
  end
...
  def self.ranks
    ['A','2','3','4','5','6','7','8','9','10','J','Q','K']
  end

  def self.suits
    ['Clubs', 'Diamonds', 'Hearts', 'Spades']
  end
...
end // class Card

[joncodo]

Instead of in the initialize, could I use validates? Or is that only for db saves?

[scott-steadman]

Primarily for classes that extend ActiveRecord::Base. You could mix in AR::Validations, but you don't seem to be using the rails framework here, so I'd advise against it.

If you think you can re-use the rank and suit checking code, you can do the following:

def initialize(rank, suit)
  ensure_correct_rank(rank)
  ensure_correct_suit(suit)
  self.rank = rank
  self.suit = suit
end

private

def ensure_correct_rank(rank)
  raise ArgumentError 'Invalid Rank' unless Card.ranks.include?(rank)
end

def ensure_correct_suit(rank)
  raise ArgumentError 'Invalid Suit' unless Card.suits.include?(suit)
end

[scott-steadman]

Card class should be immutable Reason: prevent someone from accidentally changing the rank/suit of a card

Someone may do this accidentally:

   return 1 if card.rank = 1 # should be == instead of =

Example code:

class Card
  attr_reader :rank, :suit

  def initialize(rank, suit)
    ensure_correct_rank(rank)
    ensure_correct_suit(suit)
    @rank = rank
    @suit = suit
  end
...
end // class Card