12
\$\begingroup\$

First off, here's the code:

main.m

#import <Foundation/Foundation.h>
#import "PSBoard.h"
#import "PSPlayer.h"
#import "PSInputHandler.h"

int main(int argc, const char * argv[]) {

    @autoreleasepool {

        NSLog(@"Enter Player 1 Name:");
        NSString *playerOneName = [PSInputHandler getString];
        NSLog(@"Enter Player 2 Name:");
        NSString *playerTwoName = [PSInputHandler getString];

        NSLog(@"How many rows and columns will you play with?");
        NSUInteger numberOfRowsAndColumns = [PSInputHandler getInteger];

        PSPlayer *playerOne = [[PSPlayer alloc] initWithSymbol:PSBoardSymbolX name:playerOneName];
        PSPlayer *playerTwo = [[PSPlayer alloc] initWithSymbol:PSBoardSymbolO name:playerTwoName];
        PSBoard  *board = [[PSBoard alloc] initWithRows:numberOfRowsAndColumns columns:numberOfRowsAndColumns players:@[playerOne, playerTwo]];

        do {
            PSPlayer *currentPlayer = [board playerUp];
            BOOL validInputEntered = NO;

            //Loop until valid input is entered
            while(!validInputEntered) {
                //Get input coordinates
                NSLog(@"%@, enter a row (1-%lu).", currentPlayer.name, (unsigned long)numberOfRowsAndColumns);
                NSUInteger row = [PSInputHandler getInteger];
                NSLog(@"Now enter a column (1-%lu).", (unsigned long)numberOfRowsAndColumns);
                NSUInteger column = [PSInputHandler getInteger];

                //Verify that nothing is already placed there
                PSBoardSymbol symbolOfPlayerAtCoordinates = [board playerAtRow:row-1 column:column-1].symbol;

                if((symbolOfPlayerAtCoordinates != PSBoardSymbolX && symbolOfPlayerAtCoordinates != PSBoardSymbolO) &&
                   row > 0 && row <= numberOfRowsAndColumns && column > 0 && column <= numberOfRowsAndColumns) {
                    [board setPlayer:currentPlayer atRow:(row-1) column:(column-1)];
                    validInputEntered = YES;
                }
            }

            //Show the board
            [board display];

        } while(!board.winner);

        NSLog(@"Congrats %@! You won.", [board winner].name);
    }

    return 0;
}

PSBoard.h

#import <Foundation/Foundation.h>

@class PSPlayer;

@interface PSBoard : NSObject

@property (nonatomic) NSUInteger        rows;
@property (nonatomic) NSUInteger        columns;
@property (nonatomic, strong) PSPlayer *winner;

-(instancetype)initWithRows:(NSUInteger)rows columns:(NSUInteger)columns players:(NSArray *)players;
-(PSPlayer *)playerAtRow:(NSUInteger)row column:(NSUInteger)column;
-(void)setPlayer:(PSPlayer *)player atRow:(NSUInteger)row column:(NSUInteger)column;
-(void)display;
-(PSPlayer *)playerUp;

@end

PSBoard.m

#import "PSBoard.h"
#import "PSPlayer.h"

@interface PSBoard ()

@property (nonatomic, strong) NSMutableArray *internalBoardRepresentation;
@property (nonatomic, strong) NSArray *players;
@property (nonatomic, strong) PSPlayer *oldPlayerUp;

@end

@implementation PSBoard

-(instancetype)initWithRows:(NSUInteger)rows columns:(NSUInteger)columns players:(NSArray *)players {

    if(self = [super init]) {

        self.rows = rows;
        self.columns = columns;
        self.players = players;
        self.internalBoardRepresentation = [[NSMutableArray alloc] initWithCapacity:rows];

        PSPlayer *null = [[PSPlayer alloc] initWithSymbol:PSBoardSymbolNone name:nil];

        for(NSUInteger row = 0; row < rows; row++) {

            NSMutableArray *currentColumn = [NSMutableArray array];
            for(NSUInteger column = 0; column < columns; column++) {
                [currentColumn addObject:null];
            }
            [self.internalBoardRepresentation addObject:currentColumn];
        }

        self.oldPlayerUp = players[0];
    }

    return self;
}
-(PSPlayer *)playerAtRow:(NSUInteger)row column:(NSUInteger)column {
    return self.internalBoardRepresentation[row][column];
}
-(void)setPlayer:(PSPlayer *)player atRow:(NSUInteger)row column:(NSUInteger)column {
    self.internalBoardRepresentation[row][column] = player;
    [self checkForWinner];
}
-(void)checkForWinner {

    NSUInteger numberOfPiecesInARowToWin = MAX(self.rows, self.columns);

    //Check horizontal lines
    for(NSUInteger row = 0; row < self.rows; row++) {

        PSPlayer *playerInFirstColumn = [self playerAtRow:row column:0];
        NSUInteger playerPiecesInRow = 0;

        for(NSUInteger column = 0; column < self.columns; column++) {
            if([[self playerAtRow:row column:column] isEqualTo:playerInFirstColumn]) {
                playerPiecesInRow++;
            }
        }

        if(playerPiecesInRow >= numberOfPiecesInARowToWin && playerInFirstColumn.symbol != PSBoardSymbolNone) {
            self.winner = playerInFirstColumn;
            return;
        }
    }

    //Check vertical lines
    for(NSUInteger column = 0; column < self.columns; column++) {

        PSPlayer *playerInFirstRow = [self playerAtRow:0 column:column];
        NSUInteger playerPiecesInColumn = 0;

        for(NSUInteger row = 0; row < self.rows; row++) {
            if([[self playerAtRow:row column:column] isEqualTo:playerInFirstRow]) {
                playerPiecesInColumn++;
            }
        }

        if(playerPiecesInColumn >= numberOfPiecesInARowToWin && playerInFirstRow.symbol != PSBoardSymbolNone) {
            self.winner = playerInFirstRow;
            return;
        }
    }

    //Check top left to bottom right diagonal
    PSPlayer *playerInFirstSlotOfLeftDiagonal = [self playerAtRow:0 column:0];
    NSUInteger playerPiecesInLeftDiagonal = 0;

    for(NSUInteger row = 0, column = 0; row < self.rows; column++, row++) {
        if([[self playerAtRow:row column:column] isEqualTo:playerInFirstSlotOfLeftDiagonal]) {
            playerPiecesInLeftDiagonal++;
        }
    }

    if(playerPiecesInLeftDiagonal >= numberOfPiecesInARowToWin && playerInFirstSlotOfLeftDiagonal.symbol != PSBoardSymbolNone) {
        self.winner = playerInFirstSlotOfLeftDiagonal;
        return;
    }

    //Check bottom left to top right diagonal
    PSPlayer *playerInFirstSlotOfRightDiagonal = [self playerAtRow:self.rows-1 column:0];
    NSUInteger playerPiecesInRightDiagonal = 0;

    for(NSInteger row = self.rows-1, column = 0; row >= 0; row--, column++) {
        if([[self playerAtRow:row column:column] isEqualTo:playerInFirstSlotOfRightDiagonal]) {
            playerPiecesInRightDiagonal++;
        }
    }

    if(playerPiecesInRightDiagonal >= numberOfPiecesInARowToWin && playerInFirstSlotOfRightDiagonal.symbol != PSBoardSymbolNone) {
        self.winner = playerInFirstSlotOfRightDiagonal;
        return;
    }
}
-(void)display {

    NSMutableString *displayString = [NSMutableString stringWithFormat:@"\n"];

    for(NSUInteger row = 0; row < self.rows; row++) {

        NSMutableString *rowDisplayString = [[NSMutableString alloc] init];
        NSString *innerFillerString = (row == self.rows-1) ? @" " : @"_";

        for(NSUInteger column = 0; column < self.columns; column++) {
            NSString *columnSeparator = (column == self.columns-1) ? @" " : @"|";
            NSString *playerSymbol = ([self playerAtRow:row column:column].symbolStringRepresentation);

            if(playerSymbol.length == 0) {
                playerSymbol = innerFillerString;
            }

            [rowDisplayString appendString:[NSString stringWithFormat:@"%@%@%@%@", innerFillerString, playerSymbol, innerFillerString, columnSeparator]];
        }

        [displayString appendString:[NSString stringWithFormat:@"%@\n", rowDisplayString]];
        [rowDisplayString setString:@""];
    }

    NSLog(@"%@", displayString);
}
-(PSPlayer *)playerUp {

    PSPlayer *nextPlayerUp = self.players[1-([self.players indexOfObjectIdenticalTo:self.oldPlayerUp])];
    PSPlayer *previousPlayerUp = self.oldPlayerUp;
    self.oldPlayerUp = nextPlayerUp;
    return previousPlayerUp;
}

@end

PSPlayer.h

#import <Foundation/Foundation.h>

typedef NS_ENUM(NSInteger, PSBoardSymbol) {

    PSBoardSymbolX = 0,
    PSBoardSymbolO,
    PSBoardSymbolNone
};

@interface PSPlayer : NSObject

-(instancetype)initWithSymbol:(PSBoardSymbol)symbol name:(NSString *)name;

@property (nonatomic)  PSBoardSymbol    symbol;
@property (nonatomic)  NSString        *symbolStringRepresentation;
@property (nonatomic, strong) NSString *name;

@end

PSPlayer.m

#import "PSPlayer.h"

@implementation PSPlayer

-(instancetype)initWithSymbol:(PSBoardSymbol)symbol name:(NSString *)name{

    if(self = [super init]) {

        self.symbol = symbol;
        self.symbolStringRepresentation = (symbol == PSBoardSymbolO) ? @"O" : ((symbol == PSBoardSymbolX) ? @"X" : @"");
        self.name = name;
    }

    return self;
}

@end

PSInputHandler.h

#import <Foundation/Foundation.h>

@interface PSInputHandler : NSObject

+(NSString *)getString;
+(NSInteger)getInteger;

@end

PSInputHandler.m

#import "PSInputHandler.h"

@implementation PSInputHandler

+(NSInteger)getInteger {
    int temp;
    scanf("%i", &temp);
    return (NSInteger)temp;
}
+(NSString *)getString {
    char input[256];
    scanf("%s", input);
    return [NSString stringWithUTF8String:input];
}

@end

So my questions are:

  1. In the PSInputHandler.m class, I wasn't so sure about how to get input from the command line. I read that fgets() is a potential alternative to scanf(), but is there any reason for me to use one over the other?
  2. The method in PSBoard.m that checks for a winner, checkForWinner, is very long. Is there a simplified design I can use to shorten it?
  3. I struggled to name the playerUp method, which returns the player whose turn it is. Is there a more suitable name?
  4. When the user inputs which row and column to place an X or O in, I made it so that the coordinates they enter are from 1 to the number of rows and not 0 to the number of rows minus one (like with zero-based array indexing). Is this more user-friendly, or should I change it to zero-based indexing?
  5. In PSPlayer.m, I use nested ternary operators. Is this too hard to understand? Should I change it to if statements?
    1. When getting the user's input for how many rows and columns to use, which I expect to be an integer, how can I sanitize the input so that the program doesn't crash when a string (for example) is inputted?

Any other critique welcome!

\$\endgroup\$
  • 6
    \$\begingroup\$ Hi and welcome to CodeReview. You have just passed the 'First Question' review with flying colours. Good question. You may, by the way, be interested in the current Ultimate-Tic-Tac-Toe code-challenge that is taking place this month. \$\endgroup\$ – rolfl Feb 4 '14 at 3:30
  • \$\begingroup\$ @rolfl - Thanks! Glad to see I'm not violating any rules. That challenge looks interesting. Might take a stab at it. \$\endgroup\$ – pasawaya Feb 4 '14 at 4:11
13
\$\begingroup\$

I will update this post over the weekend as I go through your question more and come up with some examples to iterate over my points, but I thought for now, I'd answer some of the easier questions.


Question 1.

I'm not sure and cannot remember (I will try to find out). At the end of the day, you might consider implementing this with a GUI. If you're using Xcode, it's quite easy to develop a GUI for either OSX or iOS, and most of your logic is already in place. You'd just have to write the logic to hook the GUI up to the business logic.


Question 2.

One immediate thought on speeding up this process would be to use a flag to mark whether or not a row/column is a potential winner. AND, if you do find a row that's a winner, immediately return the winner.

For a row to be a winner, every piece in the row must belong to the same player, correct? So set the owner of the piece in the first box, and check every box. As soon as you get to a box that doesn't match the first box, break;. You don't need to check any more boxes in that row/column/diagonal. You can move to the next row/column/diagonal. And if you get to the end of the inner loop and haven't had to break; because you've found the winner, then you can set the winner and return; and stop checking.

So basically, refactor into something more like this:

for(NSUInteger row = 0; row < self.rows; row++) {

    PSPlayer *playerInFirstColumn = [self playerAtRow:row column:0];
    BOOL winnerFound;

    for(NSUInteger column = 0; column < self.columns; column++) {
        if(![[self playerAtRow:row column:column] isEqualTo:playerInFirstColumn]) {
            winnerFound = false;
            break;
        } else {
            winnerFound = true;
        }
    }

    if (winnerFound) {
        self.winner = playerInFirstColumn;
        return;
    }
}

This will improve performance some. You can probably still do better, but this is still a drastic improvement, especially for exceptionally large boards.

Now... the BEST performance improvement I can think of would actually mean you're running this check after every turn (which you're already doing, right?). In this case, you only need to check ONE row, ONE column, and ZERO, ONE, or TWO diagonals. And this would be a massive performance boost. You only need to check a the row the piece was played in, the column the piece was played in, and the diagonal the piece was played in. Every other row, column, and diagonal has been checked on a previous turn and a winner was not found otherwise the game would be over and we wouldn't've had this turn.

AND, even if we modified the rules to continue playing after a winner has been found, you can just use an array to keep track of each row and column and diagonal and who won that row/column/diagonal, and still only need to check the relevant rows (and only check them for the player who played the piece).


Question 3.

playerUp is probably an alright method name. Maybe activePlayer? If you feel it's not descriptive enough, don't hesitate to leave a comment explaining it. // returns the player whose turn it is


Question 4.

As a programmer, I am used to 0-based indexing systems, but your assumption is correct. Most people who use programs aren't programs and would be more comfortable with a 1-based coordinate system. Though... back to question 1... if this were given a GUI, it wouldn't matter. ;)


Question 5.

Personally, I hate the ternary operators and never use them. Whether or not they're acceptable would depend largely on who you're working with though. In this case, it's a simple one. Again, personally, I hate them and I wouldn't use it, because I never use it, but this one is simple enough that if you and everyone working on the project are comfortable with them, then go ahead and keep it.

Question 5.1.

The exact way you want to handle non-number input is an implementation detail that'd be up to you. Do you want to request another input? Do you want to just strip out the non-numbers and use the numbers that are there?

But as for actually checking the string itself, once you've got it into an NSString, it's quite easy:

NSCharacterSet *nonNumbers = [[NSCharacterSet 
    decimalDigitCharacterSet] invertedSet];

if([yourString rangeOfCharactersFromSet:nonNumbers].location == NSNotFound) {
    // string is all numbers and is good to go
} else {
    // string contains non-number characters
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.