Simon's Opposites Game

I cooked up this little game today for the community challenge mentioned here. The idea of the game is that sometimes when Simon gives you a color, you should choose the opposite color (the color in the opposite corner). The game visually indicates this when it is an Opposite Simon. I'm not totally sure that the gameplay is "fun", but it was definitely fun to make!

First up, here is the game model:

SOGame.h

#import 
#import "SOGameStates.h"
#import "SOSimonSays.h"

@interface SOGame : NSObject

@property GameState state;
@property NSMutableArray *sequence;

-(void) update;
-(void) evaluateMove:(SimonSays)playerChoice;
-(void) restart;

@end

SOGame.m

```
#import "SOGame.h"
#import "SOSimon.h"

@implementation SOGame {
int _selectionNumber;
}

-(id) init {
self = [super init];
if (self) {
_state = GameStateStarted;
_sequence = [[NSMutableArray alloc]init];
_selectionNumber = 0;
}
return self;
}

#pragma mark - Game Rules
-(void) update {
switch (self.state) {
case GameStateStarted:
[self createFirstSequence];
break;
case GameStateSuccess:
[self createNewSequence];
break;
default:
break;
}
}
-(void) evaluateMove:(SimonSays)playerChoice {

//if an invalid node is selected
if (playerChoice == SimonSaysNumChoices) {
return;
}

SOSimon *simon = [_sequence objectAtIndex:_selectionNumber];

//different rules for opposite versus normal simons
if (simon.opposite) {
if (playerChoice == [self opposititePosition:simon.simonSays]) {
[self doSuccessfulChoice];
} else {
_state = GameStateFailure;
}
} else {
if (playerChoice == simon.simonSays) {
[self doSuccessfulChoice];
} else {
_state = GameStateFailure;
}
}
}
-(void) doSuccessfulChoice {
if (_selectionNumber < _sequence.count - 1) {
_

@property NSMutableArray *sequence;

So, I want to just look at this property on our class. As it stands, it's a publicly available property on our class. There's no reason someone couldn't come along and write this code:

[soGameInstance.sequence removeAllObjects];
[soGameInstance.sequence addObject:[NSNull null]];
[soGameInstance evaluateMove:SimonSaysTopLeft];

And now you're getting an exception that sounds something like this:


NSNull does not respond to selector 'getSimonSays'

I've only skimmed through the rest of your code (because this publicly exposed mutable array bothers me that much), so I'm not sure how exactly this array is accessed, if at all, by your current code, but does it even need to be?

We do need an internal array to keep track of the move sequence, that is true. But, in terms of external access to that array?

We could offer a readonly immutable array:

@property (readonly) NSArray *sequence;

Which does nothing more than return a copy of the internal sequence array:

- (NSArray *)sequence {
     return [NSArray arrayWithArray:_sequence];
}

(Where we've declared NSMutableArray *_sequence; in our .m file).

OR, I might like it better if we just offer a method for fetching a particular sequence number:

- (SOSimon *)simonSaysAtIndex:(NSUInteger)index {
    return (_sequence.count > index) ? _sequence[index] : nil;
}

One other small note:

-(NSMutableArray *) sequence:(NSMutableArray *)oldSequence {
    NSMutableArray *newSequence = [[NSMutableArray alloc]init];
    if (oldSequence) {
        newSequence = [[NSMutableArray alloc]initWithArray:oldSequence];
    }
    [newSequence addObject:[[SOSimon alloc]init]];
    return newSequence;
}

Here, we're unnecessarily double allocating an array any time oldSequence is non-nil. We should instead do this:

NSMutableArray *newSequence;
if (oldSequence) {
    newSequence = [NSMutableArray arrayWithArray:oldSequence];
} else {
    newSequence = [NSMutableArray array];
}
[newSequence addObject:[[SOSimon alloc] init]];
return newSequence;

Although, why can't we just do this?

if (!oldSequence) {
    oldSequence = [NSMutableArray array];
}
[oldSequence addObject:[[SOSimon alloc] init]];
return oldSequence;

Either way, this method actually seems a little strange.

StackExchange Code Review Q#71819, answer score: 4