Cards Class

P

Peter Marsh

Hi, I'm an utter newbie to Ruby (and OOP!) and I thought I'd share my
very first project. It's nothing special - just a simple class that
simulates a deck of cards, but I'd like to get your comments to see if
there's anything I can do better (enjoy!):

class Deck
@@Cards =
['Two','Three','Four','Five','Six','Seven','Eight','Nine','Ten','Jack','Queen','King','Ace']
@@Suits = ['Spades','Hearts','Diamonds','Clubs']

def initialize
@top_card = -1
@deck = []
52.times do |card|
@deck << card
end
end

def draw
@top_card = @top_card + 1
if @top_card < 52
@deck[@top_card]
else
raise 'There are only 52 cards in a deck!'
end
end

def shuffle
@top_card = -1
@deck = @deck.sort_by {rand}
end

def suit
@@Suits[@top_card/13]
end

def face
factor = @top_card/13
index = @top_card - 13*factor
@@Cards[index]
end

def set_card(card)
@top_card=card
end

end
 
J

Joseph Seaton

Nice work! I'm an utter newbie (probably more of one than you) but
since you asked for comments...

Peter said:
Hi, I'm an utter newbie to Ruby (and OOP!) and I thought I'd share my
very first project. It's nothing special - just a simple class that
simulates a deck of cards, but I'd like to get your comments to see if
there's anything I can do better (enjoy!):

class Deck
@@Cards =
['Two','Three','Four','Five','Six','Seven','Eight','Nine','Ten','Jack','Queen','King','Ace']
@@Suits = ['Spades','Hearts','Diamonds','Clubs']

def initialize
@top_card = -1
@deck = []
52.times do |card|
@deck << card
end
@deck = (0..52).to_a
end

def draw
@top_card = @top_card + 1
@top_card += 1
if @top_card < 52
@deck[@top_card]
else
raise 'There are only 52 cards in a deck!'
end
end

def shuffle
@top_card = -1
@deck = @deck.sort_by {rand}
end
Nice trick there by the way, I'll have to remember that one
def suit
@@Suits[@top_card/13]
end

def face
factor = @top_card/13
index = @top_card - 13*factor
@@Cards[index]
end

def set_card(card)
@top_card=card
end
attr_writer :top_card would be simpler
I look forward to reading your code when you nolonger consider yourself
a ruby newbie

Joe
 
R

Robert Dober

Hi, I'm an utter newbie to Ruby (and OOP!) and I thought I'd share my
very first project. It's nothing special - just a simple class that
simulates a deck of cards, but I'd like to get your comments to see if
there's anything I can do better (enjoy!):
This is indeed not a bad attempt at all, however ;)
class Deck
@@Cards =
['Two','Three','Four','Five','Six','Seven','Eight','Nine','Ten','Jack','Queen','King','Ace']
@@Suits = ['Spades','Hearts','Diamonds','Club']
Personally I dislike class variables (I prefer class instance
variables even if the make the code more clumpsy (clumpsier?)) but
here it seems to me that constants would be in order, the name of the
cards and suits are constant in the domain after all.

Cards = %w{ Deuce Three Four Five ...
Suits = %w{ Spades Hearts Diamonds Clubs }
def initialize
@top_card = -1
@deck = []
52.times do |card|
@deck << card
end
I will play my personal Ace of Trumps here
@deck = [*0..51]
end

def draw
@top_card = @top_card + 1 @top_card += 1
if @top_card < 52
@deck[@top_card]
else
raise 'There are only 52 cards in a deck!'
end
@deck[@top_card] or raise "There are only 42 cards in the deck ;)"
end

def shuffle
@top_card = -1
@deck = @deck.sort_by {rand}
end

def suit
@@Suits[@top_card/13]
end

def face
factor = @top_card/13
index = @top_card - 13*factor
@@Cards[index]
replace the three by
Cards[ @top_card % 13 ]

The above two methods seem wrong to me, is it not
@deck[@top_card]
that you want?
def set_card(card)
@top_card=card
end
No idea what that should do? If it were not for this method I would
get rid of @top_card at all in your code and write it as follows

def initialize; shuffle end
def shuffle; @deck = [*0..51].sort_by{ rand } end
def draw; @deck.shift or raise "Error only 42 cards" end
def face; Cards[@deck.first % 13] end
def suit; Suits[@deck.first / 13] end


HTH
Robert
 
J

Jesús Gabriel y Galán

Hi, I'm an utter newbie to Ruby (and OOP!) and I thought I'd share my
very first project. It's nothing special - just a simple class that
simulates a deck of cards, but I'd like to get your comments to see if
there's anything I can do better (enjoy!):

Others have commented your code, so I'll just share a Deck class I
made for some little things. It may give you some ideas or maybe
someone else will comment on it. It can contain any object, not just
normal cards. Also I want to implement some shuffle methods that
resembled a human shuffle (with it's fuzziness) apart from the actual
random sorting, but never got to do it, I just implemented a fuzzy cut
in half. Here it is:

require 'emptydeckerror'



class Deck



def initialize

@deck = []

end



def << (item)

@deck << item

self

end



def draw

item = @deck.shift

raise EmptyDeckError, "The deck is empty" if !item

item

end



def cut!(fuzzy = false)

cutpoint = @deck.size / 2 - 1

cutpoint = fuzzy(cutpoint) if fuzzy

half = @deck.slice!(0..cutpoint)

(@deck << half).flatten!

self

end



def randomize!

@deck = @deck.sort_by {rand}

self

end



def to_s

@deck.to_s

end



def empty?

@deck.size == 0

end



protected

def fuzzy(number)

# Deviation: 10% of the deck size (min 1)

deviation = @deck.size / 20

deviation = 1 if deviation == 0

randmax = deviation * 2 + 1

number + rand(randmax) - deviation

end

end

Jesus.
 
P

Peter Marsh

@Everyone

Thanks for the positive feedback/ideas :D
Robert said:
This is indeed not a bad attempt at all, however ;)

...

HTH
Robert

There are a lot of things in your post that're really great and have
helped develop my (still limited) knowledge of Ruby.

I don't quite understand this, however:
Cards = %w{ Deuce Three Four Five ...
Suits = %w{ Spades Hearts Diamonds Clubs }

From what I've been able to work out it converts everything between {}
into a string and then sticks that all in an array that Cards or Suits
points to. Is this correct?

Also
@deck[@top_card] or raise "There are only 42 cards in the deck ;)"

I understand what this means, but I'm unsure how to integrate this into
my code.

Thanks for your help!

P.S. I implimented the whole '@top_card' system so that deck[] would not
have to be refilled with numbers each time it was shuffled - I had
worries about speed but perhaps in this case they were a bit to extreme.
 
R

Robert Dober

@Everyone
Also

@deck[@top_card] or raise "There are only 42 cards in the deck ;)"

I understand what this means, but I'm unsure how to integrate this into
my code.
If I recall correctly your code was something like

if @top_card < 52 then
@deck[@top_card]
else
raise ...
end

if @top_card >=52 @deck[@top_card] evaluates to nil, as cards in the
deck are never false or nil the RHS part of the expression

@deck[@top_card] or raise ...

will be executed iff @top_card >=52.
Otherwise the LHS of the expression will be returned, exactly what you wanted.

This is very concise code, maybe even bad for readability, but worth
learning I guess.

Cheers
Robert
 
D

David Chelimsky

As a newcomer, you are in a perfect position to learn how to properly
document your work. Not only "proper" in your eyes, but it is so easy
to make it compatible with rdoc. For instance:

##
# Has properties resembling that of a deck of cards
#
Class Deck

##
# Constructs and returns an instance of Deck
#
def initialize
.......
end

##
# Does some action to an instance of Deck. Argument foo must be
of type string.....
#
def bar(foo=nil)
........
end

end

Running this through rdoc will quickly create some helpful results,
especially as your code grows and you begin to reuse as much as
possible.

Peter - Agile thinking has some interesting ideas about comments that
you should read up on as well before you dive in and comment every
little thing in your code. Essentially, comments are an additional
maintenance burden (you have to change them when you change your
code), and they become a liability if they stray from reality (which
they often do). While there are definitely times when comments are
useful, there are also times when they add very little or no value, in
which case you should prefer well named classes and methods that tell
their own story.

You can find a plethora of varying opinions about this if you google
"code smell comments".

Cheers,
David
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top