Object Oriented Card Game

[ryansobol]

I found a great website (http://nifty.stanford.edu/2004/EstellCardGame/) about creating a GUI-based card game using OO principals and Java. I'm personally making the leap into Objective-C / Cocoa and I thought it would be a great exercise to port the project.

Since I'm pretty new with Objective-C, I'd love to get some feedback from the community on my first class, Rank. Before looking over the code, it might be a good idea to read the author's write-up (http://nifty.stanford.edu/2004/EstellCardGame/assignments/cardgamepart1.html) for the game's foundation classes and the original Rank class in Java (http://nifty.stanford.edu/2004/EstellCardGame/appletdemos/Rank.java) .


Rank.h
#import <Cocoa/Cocoa.h>

@interface Rank : NSObject
{
NSString *name;
NSString *symbol;
}

+(void)initialize;

+(Rank *)ace;
+(Rank *)two;
+(Rank *)three;
+(Rank *)four;
+(Rank *)five;
+(Rank *)six;
+(Rank *)seven;
+(Rank *)eight;
+(Rank *)nine;
+(Rank *)ten;
+(Rank *)jack;
+(Rank *)queen;
+(Rank *)king;

-(Rank *)initWithName:(NSString *)aName Symbol:(NSString *)aSymbol;

-(NSString *)name;
-(NSString *)symbol;

-(void)setAceHigh;
-(void)setKingHigh;
-(int)compareTo:(Rank *)aRank;

-(NSString *)description;

@end


Rank.m
#import "Rank.h"

static BOOL aceHigh = NO;

static Rank *ACE = nil;
static Rank *TWO = nil;
static Rank *THREE = nil;
static Rank *FOUR = nil;
static Rank *FIVE = nil;
static Rank *SIX = nil;
static Rank *SEVEN = nil;
static Rank *EIGHT = nil;
static Rank *NINE = nil;
static Rank *TEN = nil;
static Rank *JACK = nil;
static Rank *QUEEN = nil;
static Rank *KING = nil;

static NSArray *VALUES_KING_HIGH = nil;
static NSArray *VALUES_ACE_HIGH = nil;

@implementation Rank


+(void)initialize
{
@synchronized(self)
{
if (ACE == nil)
{
ACE = [[Rank alloc] initWithName:@"Ace" Symbol:@"A"];
}

if (TWO == nil)
{
TWO = [[Rank alloc] initWithName:@"TWO" Symbol:@"2"];
}

if (THREE == nil)
{
THREE = [[Rank alloc] initWithName:@"Three" Symbol:@"3"];
}

if (FOUR == nil)
{
FOUR = [[Rank alloc] initWithName:@"Four" Symbol:@"4"];
}

if (FIVE == nil)
{
FIVE = [[Rank alloc] initWithName:@"Five" Symbol:@"5"];
}

if (SIX == nil)
{
SIX = [[Rank alloc] initWithName:@"Six" Symbol:@"6"];
}

if (SEVEN == nil)
{
SEVEN = [[Rank alloc] initWithName:@"Seven" Symbol:@"7"];
}

if (EIGHT == nil)
{
EIGHT = [[Rank alloc] initWithName:@"Eight" Symbol:@"8"];
}

if (NINE == nil)
{
NINE = [[Rank alloc] initWithName:@"Nine" Symbol:@"9"];
}

if (TEN == nil)
{
TEN = [[Rank alloc] initWithName:@"Ten" Symbol:@"10"];
}

if (JACK == nil)
{
JACK = [[Rank alloc] initWithName:@"Jack" Symbol:@"J"];
}

if (QUEEN == nil)
{
QUEEN = [[Rank alloc] initWithName:@"Queen" Symbol:@"Q"];
}

if (KING == nil)
{
KING = [[Rank alloc] initWithName:@"King" Symbol:@"K"];
}

if (VALUES_KING_HIGH == nil)
{
VALUES_KING_HIGH = [NSArray arrayWithObjects:
ACE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT
, NINE, TEN, JACK, QUEEN, KING, nil];
}

if (VALUES_ACE_HIGH == nil)
{
VALUES_ACE_HIGH = [NSArray arrayWithObjects:
TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT
, NINE, TEN, JACK, QUEEN, KING, ACE, nil];
}
}
}

+(Rank *)ace
{
if (ACE == nil)
{
[self initialize];
}

return ACE;
}

+(Rank *)two
{
if (TWO == nil)
{
[self initialize];
}

return TWO;
}

+(Rank *)three
{
if (THREE == nil)
{
[self initialize];
}

return THREE;
}

+(Rank *)four
{
if (FOUR == nil)
{
[self initialize];
}

return FOUR;
}

+(Rank *)five
{
if (FIVE == nil)
{
[self initialize];
}

return FIVE;
}

+(Rank *)six
{
if (SIX == nil)
{
[self initialize];
}

return SIX;
}

+(Rank *)seven
{
if (SEVEN == nil)
{
[self initialize];
}

return SEVEN;
}

+(Rank *)eight
{
if (EIGHT == nil)
{
[self initialize];
}

return EIGHT;
}

+(Rank *)nine
{
if (NINE == nil)
{
[self initialize];
}

return NINE;
}

+(Rank *)ten
{
if (TEN == nil)
{
[self initialize];
}

return TEN;
}

+(Rank *)jack
{
if (JACK == nil)
{
[self initialize];
}

return JACK;
}

+(Rank *)queen
{
if (QUEEN == nil)
{
[self initialize];
}

return QUEEN;
}

+(Rank *)king
{
if (KING == nil)
{
[self initialize];
}

return KING;
}

-(Rank *)initWithName:(NSString *)aName Symbol:(NSString *)aSymbol
{
self = [super init];

if (self != nil)
{
if (name != aName)
{
[name release];
name = [aName retain];
}

if (symbol != aSymbol)
{
[symbol release];
symbol = [aName retain];
}
}

return self;
}

-(NSString *)name
{
return name;
}

-(NSString *)symbol
{
return symbol;
}

-(void)setAceHigh
{
@synchronized(self)
{
aceHigh = YES;
}
}

-(void)setKingHigh
{
@synchronized(self)
{
aceHigh = NO;
}
}

-(int)compareTo:(Rank *)aRank
{
@synchronized(self)
{
if (aceHigh == YES)
return [VALUES_ACE_HIGH indexOfObject: self] - [VALUES_ACE_HIGH indexOfObject: aRank];
else
return [VALUES_KING_HIGH indexOfObject: self] - [VALUES_KING_HIGH indexOfObject: aRank];
}
}

-(NSString *)description
{
return [self name];
}

-(void)dealloc
{
[super dealloc];

[name release];
[symbol release];
}

// -------------- Singleton overrides --------------
// http://developer.apple.com/documentation/Cocoa/Conceptual/CocoaObjects/Articles/CreateSingleton.html

+ (id)allocWithZone:(NSZone *)zone
{
return nil;
}


- (id)copyWithZone:(NSZone *)zone
{
return self;
}

- (id)retain
{
return self;
}

- (unsigned)retainCount
{
return UINT_MAX; //denotes an object that cannot be released
}

- (void)release
{
//do nothing
}

- (id)autorelease
{
return self;
}

@end


Thanks for your help!

[skyhawk]

it looks a bit excessive for cards

[ryansobol]

Well, the author's programming challenge was to create a foundation of reusable classes for any and all card games. Anyone can implement a game-specific deck of cards using primitive types like int or char. If you have any real suggestions on "trimming" fat, I'm all ears.

[kelvin]

a bit too much redundancy.

1) +(void)initialize; only gets called once. (just in case though for whatever future odd reason) We use a static BOOL to keep track of the first initialize and do no further initializes.

2) if your cards are already instantiated in +(void)initialize; then you don't need to check for their nonexistence in your shared-object methods.

oh, and..
3) NEVER EVER call "[self initialize]"!!! This is a class method and is handled by the runtime. Your usage here is plain wrong.

so in closing:
1) make a static BOOL initializedflag variable and set that in your +(void)initialize. Remember that +(void)initialize only gets called once (and NOT by you).

2) change all the shared-object class methods to just return ACE; (or whatever it's supposed to return. When these get called +initialize has already been called (guaranteed) and your objects should exist for use (do your error checking and validity in +initialize).

3) I'm not sure why you are overriding your memory management methods, as with good memory management you'll never ever really need to bother with overriding these. (Unless of course you haven't learned Cocoa memory management in which case, post to the board and prepare for [constructive] flaming.)

[ryansobol]

1) +(void)initialize; only gets called once. (just in case though for whatever future odd reason) We use a static BOOL to keep track of the first initialize and do no further initializes.)

That makes sense, but what should the class do if a programmer accidentally calls the +ace, +two, etc. method before they've called +initialize? Return nil?


2) if your cards are already instantiated in +(void)initialize; then you don't need to check for their nonexistence in your shared-object methods.)

Okay.

oh, and..
3) NEVER EVER call "[self initialize]"!!! This is a class method and is handled by the runtime. Your usage here is plain wrong.

Thanks for the warning.

3) I'm not sure why you are overriding your memory management methods, as with good memory management you'll never ever really need to bother with overriding these. (Unless of course you haven't learned Cocoa memory management in which case, post to the board and prepare for [constructive] flaming.)

Actually, I got the idea for overriding the memory management methods from Apple's developer website on singletons (http://developer.apple.com/documentation/Cocoa/Conceptual/CocoaObjects/Articles/CreateSingleton.html). It seems like Apple treats singletons as an exception to the reference counting paradigm. I could be wrong though.

[Steven]

There is absolutely no way they can call +ace or +two, etc, before +initialize because the runtime calls it when the class is loaded into memory (right?)

[kelvin]

There is absolutely no way they can call +ace or +two, etc, before +initialize because the runtime calls it when the class is loaded into memory (right?) Yep.

like I said NEVER EVER put "[something initialize]" anywhere. Runtime does it (and always before you ever have a chance to do anything else I might add).

[ryansobol]

Yep.

like I said NEVER EVER put "[something initialize]" anywhere. Runtime does it (and always before you ever have a chance to do anything else I might add).

Wait, are you saying that the runtime system actually sends an initialize message to all class objects before the application starts?

[Steven]

There's no guarantee as to when it will be called, only that it will be before you can use the class. If the class is there at application start, then yes it would be called during the application startup.

[ryansobol]

There's no guarantee as to when it will be called, only that it will be before you can use the class. If the class is there at application start, then yes it would be called during the application startup.

I guess I'll have to take your word for it but I'd like to see the documentation on that. The only reason is because I (think) I've read most of Apple's documentation on Objective-C and memory management. Do you have a link to anything official about this?

[Steven]

No, that was just what made sense to me - you could cook up a test case...

[kelvin]

It's all specified here (http://developer.apple.com/documentation/Cocoa/Reference/Foundation/ObjC_classic/Classes/NSObject.html#//apple_ref/doc/uid/20000050/initialize). :wacko:

[ryansobol]

It's all specified here (http://developer.apple.com/documentation/Cocoa/Reference/Foundation/ObjC_classic/Classes/NSObject.html#//apple_ref/doc/uid/20000050/initialize). :wacko:

I stand corrected. I feel alot more confident using the +initialize technique for singletons in ObjC. Thanks!

[ryansobol]

After taking everyone's suggestions into consideration, I've updated and re-posted the Rank class.

Rank.h
#import <Cocoa/Cocoa.h>


@interface Rank : NSObject
{
NSString *name;
NSString *symbol;
}

+(Rank *)ace;
+(Rank *)two;
+(Rank *)three;
+(Rank *)four;
+(Rank *)five;
+(Rank *)six;
+(Rank *)seven;
+(Rank *)eight;
+(Rank *)nine;
+(Rank *)ten;
+(Rank *)jack;
+(Rank *)queen;
+(Rank *)king;
+(NSEnumerator *)rankEnumerator;

-(Rank *)initWithName:(NSString *)aName Symbol:(NSString *)aSymbol;

-(NSString *)name;
-(NSString *)symbol;

-(void)setAceHigh;
-(void)setKingHigh;
-(int)compareTo:(Rank *)aRank;

@end

Rank.m
#import "Rank.h"

static BOOL initialized = NO;
static BOOL aceHigh = NO;

static Rank *ACE = nil;
static Rank *TWO = nil;
static Rank *THREE = nil;
static Rank *FOUR = nil;
static Rank *FIVE = nil;
static Rank *SIX = nil;
static Rank *SEVEN = nil;
static Rank *EIGHT = nil;
static Rank *NINE = nil;
static Rank *TEN = nil;
static Rank *JACK = nil;
static Rank *QUEEN = nil;
static Rank *KING = nil;

static NSArray *VALUES_KING_HIGH = nil;
static NSArray *VALUES_ACE_HIGH = nil;

@implementation Rank


+(void)initialize
{
if (initialized == NO)
{
initialized = YES;

ACE = [[Rank alloc] initWithName:@"Ace" Symbol:@"A"];
TWO = [[Rank alloc] initWithName:@"TWO" Symbol:@"2"];
THREE = [[Rank alloc] initWithName:@"Three" Symbol:@"3"];
FOUR = [[Rank alloc] initWithName:@"Four" Symbol:@"4"];
FIVE = [[Rank alloc] initWithName:@"Five" Symbol:@"5"];
SIX = [[Rank alloc] initWithName:@"Six" Symbol:@"6"];
SEVEN = [[Rank alloc] initWithName:@"Seven" Symbol:@"7"];
EIGHT = [[Rank alloc] initWithName:@"Eight" Symbol:@"8"];
NINE = [[Rank alloc] initWithName:@"Nine" Symbol:@"9"];
TEN = [[Rank alloc] initWithName:@"Ten" Symbol:@"10"];
JACK = [[Rank alloc] initWithName:@"Jack" Symbol:@"J"];
QUEEN = [[Rank alloc] initWithName:@"Queen" Symbol:@"Q"];

VALUES_KING_HIGH = [NSArray arrayWithObjects:
ACE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT
, NINE, TEN, JACK, QUEEN, KING, nil];

VALUES_ACE_HIGH = [NSArray arrayWithObjects:
TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT
, NINE, TEN, JACK, QUEEN, KING, ACE, nil];
}
}

+(Rank *)ace
{
return ACE;
}

+(Rank *)two
{
return TWO;
}

+(Rank *)three
{
return THREE;
}

+(Rank *)four
{
return FOUR;
}

+(Rank *)five
{
return FIVE;
}

+(Rank *)six
{
return SIX;
}

+(Rank *)seven
{
return SEVEN;
}

+(Rank *)eight
{
return EIGHT;
}

+(Rank *)nine
{
return NINE;
}

+(Rank *)ten
{
return TEN;
}

+(Rank *)jack
{
return JACK;
}

+(Rank *)queen
{
return QUEEN;
}

+(Rank *)king
{
return KING;
}

+(NSEnumerator *)rankEnumerator
{
return [VALUES_KING_HIGH objectEnumerator];
}

-(Rank *)initWithName:(NSString *)aName Symbol:(NSString *)aSymbol
{
self = [super init];

if (self != nil)
{
if (name != aName)
{
[name release];
name = [aName retain];
}

if (symbol != aSymbol)
{
[symbol release];
symbol = [aName retain];
}
}

return self;
}

-(NSString *)name
{
return name;
}

-(NSString *)symbol
{
return symbol;
}

-(void)setAceHigh
{
@synchronized(self)
{
aceHigh = YES;
}
}

-(void)setKingHigh
{
@synchronized(self)
{
aceHigh = NO;
}
}

-(int)compareTo:(Rank *)aRank
{
@synchronized(self)
{
if (aceHigh == YES)
return [VALUES_ACE_HIGH indexOfObject: self] - [VALUES_ACE_HIGH indexOfObject: aRank];
else
return [VALUES_KING_HIGH indexOfObject: self] - [VALUES_KING_HIGH indexOfObject: aRank];
}
}

-(NSString *)description
{
return [self name];
}

-(void)dealloc
{
[super dealloc];

[name release];
[symbol release];
}

// -------------- Singleton overrides --------------
// http://developer.apple.com/documentation/Cocoa/Conceptual/CocoaObjects/Articles/CreateSingleton.html

+ (id)allocWithZone:(NSZone *)zone
{
return nil;
}


- (id)copyWithZone:(NSZone *)zone
{
return self;
}

- (id)retain
{
return self;
}

- (unsigned)retainCount
{
return UINT_MAX; //denotes an object that cannot be released
}

- (void)release
{
//do nothing
}

- (id)autorelease
{
return self;
}

@end


As you can see, I've fixed all the class methods to conform to the runtime system better. I've also added the +rankEnumerator method that will return an NSEnumerator * object for easy iteration of all the static Rank objects. The original Java class used a static List object for iteration, but I think my technique is a little more standard in Cocoa/ObjC.

Thanks for all of your feedback so far! I'd love to hear more critiques for the community on this class.

[kelvin]

better...

but, there's still some little kinks.

The instance method -init... should only be ever called once per alloc'd instance (this is up to you and your code). (This is also why most Cocoa programming tutorials suggest you nest the alloc/inits.) So therefore, you shouldn't ever have to check the instance members against nil, just set them. When you alloc an object it's instance members are all nil/NULL.

also, in your dealloc method...
[super dealloc]; should always be the last line of your subclass' dealloc method. Calling it first like you do can release important stuff and cause serious problems. Do your subclass member clean up, then pass to the superclass to do the final clean up.

[ryansobol]

better...

but, there's still some little kinks.

The instance method -init... should only be ever called once per alloc'd instance (this is up to you and your code). (This is also why most Cocoa programming tutorials suggest you nest the alloc/inits.) So therefore, you shouldn't ever have to check the instance members against nil, just set them. When you alloc an object it's instance members are all nil/NULL.

I'm assuming you're referencing the instance variables being set in -(Rank *)initWithName: Symbol: method. I think I got the getter/setter technique confused with the alloc/init technique. Here's the method now.

-(Rank *)initWithName:(NSString *)aName Symbol:(NSString *)aSymbol
{
self = [super init];

if (self != nil)
{
name = [aName retain];
symbol = [aSymbol retain];
}

return self;
}

also, in your dealloc method...
[super dealloc]; should always be the last line of your subclass' dealloc method. Calling it first like you do can release important stuff and cause serious problems. Do your subclass member clean up, then pass to the superclass to do the final clean up.

My bad. Here's the new -dealloc method.

-(void)dealloc
{
[name release];
[symbol release];

[super dealloc];
}

[blobbo]

I dunno, but doesn't this seem a little overkill? I'm interested, because you've managed to make it act pretty "english" (i.e. no cryptic arrays of cards, etc). But I would have never thought to do it this way... Neat, though. Keep it up.

[ryansobol]

Honestly, I wouldn't have thought about creating complete classes to represent primative aspects of a playing card. Originally, I thought about using a C enumeration or possibly a C struct. After I stumbled across the programming challenge, I really enjoyed the author's explanation of the problem and the typical pit-falls made by novice programmers. If you get a chance, read the article that I included in my first post. It really helped me understand some of the decisions to 'overkill' the problem.

[ryansobol]

After running a few tests, I'm finding that all my nice static class objects aren't being initialized properly. I ran the gdb and realized that the +initialize method is NOT sending messages to the -initWithName: Symbol: method at all. This leads me to believe that +initialize methods cannot send messages to instance methods of the same class. I've googled for more information on this, but so far, nothing definitive has been said about using custom instance methods inside the +initialize method. Anyone have a clue as to what might be going on?

[kelvin]

try changing...
ACE = [[Rank alloc] initWithName:@"Ace" Symbol:@"A"];
to...
ACE = [[self alloc] initWithName:@"Ace" Symbol:@"A"];
in your +initialize classmethod.

[kelvin]

and get rid of those allocWithZone, etc. overrides. If you're returning nil from allocWithZone, you're not gonna be able to allocate anything in the first place.

[Steven]

Don't override the basic release, retain, autorelease mechanism unless you have a really good reason. Instead, just ensure that you retain it once when its created and nobody releases it.