Tic Tac Toe in Swift

Any feedback encouraged. My checkForWinner function feels cumbersome to me.

```
import UIKit

class ViewController: UIViewController {

@IBOutlet weak var buttonOne: UIButton!
@IBOutlet weak var buttonTwo: UIButton!
@IBOutlet weak var buttonThree: UIButton!
@IBOutlet weak var buttonFour: UIButton!
@IBOutlet weak var buttonFive: UIButton!
@IBOutlet weak var buttonSix: UIButton!
@IBOutlet weak var buttonSeven: UIButton!
@IBOutlet weak var buttonEight: UIButton!
@IBOutlet weak var buttonNine: UIButton!

@IBOutlet weak var gameResult: UILabel!

var gameState = ["","","",
"","","",
"","",""
]

let winningCombos = [[0, 1, 2], [3, 4, 5], [6, 7, 8], //horizontal
[0, 3, 6], [1, 4, 7], [2, 5, 8], //vertical
[0, 4, 8], [2, 4, 6]] //diagonals

var gameOver = false
var turnCount = 0
var currentPlayer: String!

func pickFirstPlayer() -> String {
if arc4random_uniform(2) == 0 {
currentPlayer = "o"
} else {
currentPlayer = "x"
}
return currentPlayer
}

func toggleNextPlayer() {
if currentPlayer == "o" {
currentPlayer = "x"
} else {
currentPlayer = "o"
}
}

@IBAction func makeMove(sender: AnyObject) {
let space = sender.tag
if (gameState[space] == "" && gameOver == false) {
let turnImage = UIImage(named: "\(self.currentPlayer).png")
sender.setImage(turnImage!, forState: .Normal)
gameState[space] = self.currentPlayer
self.toggleNextPlayer()
self.turnCount++
self.checkForWinner()
}
}

func checkForWinner() {
var winner = ""
for combo in winningCombos {
var row: [String] = []
for space in combo {
row.append(gameState[space])

Using strings (in your case: "o" and "x") to identify the two
players is inflexible and error-prone:

• If you mistype "o" as "O" or "0" then the program will not work

properly or crash, and the compiler cannot warn you about that.

• If you decide to use different player names or different names for the image resources then you have to change it at many places.

-
The actual display of a player's name as uppercase letter is "hidden"
at

winner = row[0].uppercaseString

in checkForWinner().

Defining string constants

let playerO = "o"
let playerX = "x"

would be one possible solution, but the better way is to use an
enumeration which can take exactly two values:

enum Player : String {
    case X = "x"
    case O = "o"
}

Methods to get the "other" player, the display image, and to select
a random player can be added directly to the enum definition:

enum Player : String {
    case X = "x"
    case O = "o"

    func other() -> Player {
        switch (self) {
        case .O: return .X
        case .X: return .O
        }
    }

    func image() -> UIImage {
        return UIImage(named: self.rawValue + ".png")!
    }

    static func random() -> Player {
        return arc4random_uniform(2) == 0 ? .O : .X
    }
}

This makes the pickFirstPlayer and toggleNextPlayer() methods
in the view controller obsolete, and viewDidLoad() becomes

override func viewDidLoad() {
    super.viewDidLoad()
    currentPlayer = Player.random()
}

where currentPlayer is defined as

var currentPlayer: Player!

To print a player's name, implement the CustomStringConvertible
protocol:

extension Player : CustomStringConvertible {
    var description: String { 
        return self.rawValue.uppercaseString
    }
}

If winner is value of type Player then

gameResult.text = "\(winner) is the winner!"

prints the winner's name using the description method, and you
can adopt that to your needs without affecting the rest of the code or the image resource names.

Your gameState variable describes the contents of the game board,
so I would call it board instead. Each field can be empty, or be
occupied by one of the players. This can be modeled by

var board : [Player?] = [nil, nil, nil,
    nil, nil, nil,
    nil, nil, nil,
]

i.e. an array of optional players.

I would separate the determination of a possible winner from the
display of the outcome, e.g. by defining the method as

func checkForWinner() -> Player? { ... }

which either returns the winner or nil. In that method,

var row: [String] = []
        for space in combo {
            row.append(gameState[space])
        }

can be simplified using map(). Using the new Player type and return type it would look this:

func checkForWinner() -> Player? {
    for combo in winningCombos {
        let row = combo.map { board[$0] }
        if (row[0] != nil && row[0] == row[1] && row[1] == row[2]) {
            return row[0]
        }
    }
    return nil
}

It would suffice to check only the rows which are changed by the
last move. On the other hand, with 8 winning combinations, that
does not make a big performance difference.

Assuming that the makeMove() action is connected to buttons
only, you should declare it as

@IBAction func makeMove(sender: UIButton) { ... }

this allows better error checking than sender : AnyObject.
(When you create the action in interface builder, there is an option
for that.)

Using all the above stuff, the makeMove() method becomes

@IBAction func makeMove(sender: UIButton) {
    let space = sender.tag
    if (board[space] == nil && !gameOver) {

        sender.setImage(currentPlayer.image(), forState: .Normal)
        board[space] = currentPlayer
        turnCount++

        if let winner = checkForWinner() {
            gameOver = true
            gameResult.text = "\(winner) is the winner!"
        } else if (turnCount == 9) {
            gameOver = true
            gameResult.text = "It's a tie!"
        } else {
            currentPlayer = currentPlayer.other()
        }
    }
}

You use the UIView's tag property to identify from which button
the action method is called. This is OK, but note that 0 (zero)
is the default tag for all views. It is therefore better to tag the
buttons with 1 ... 9 instead of 0 ... 8, and adjust that in

let space = sender.tag - 1

A final note about using self: You can access properties and
methods of the current class without using self (unless there
is an ambiguity, e.g. due to a local variable or function parameter with the name name, or if self is captured implicitly in a block.)

Whether you use self or not to access properties is
a matter of taste and programming style,
but in any case it should be consistent in your code, which it
currently isn't:

self.turnCount++
// ...
gameOver = true

(Remark: There was a language proposal to

StackExchange Code Review Q#117438, answer score: 3