iOS Memory Game - Swift 4

Question

I'm learning iOS through this past Stanford CS193P course. I've just completed the first assignment and would love a code review as I can't actually turn in an assignment for feedback since the course has already completed.

The requirements can be found here but any suggestions on best practices and my implantation would be helpful. The app works and is in a state I would submit if I could. I'm just looking for another set of eyes.

GitHub

 import Foundation
 /**
 a game card that contains the following var:
 - isFaceUp
 - isMatched
 - identifier
 */
 struct Card: Hashable
 {
 var hashValue: Int {
 return identifier
 }
 static func == (lhs: Card, rhs: Card) -> Bool {
 return lhs.identifier == rhs.identifier
 }
 var isFaceUp = false
 var isMatched = false
 private var identifier: Int
 private static var identifierFactory = 0
 private static func getUniqueIdentifer() -> Int {
 identifierFactory += 1
 return identifierFactory
 }
 init(){
 self.identifier = Card.getUniqueIdentifer()
 }
 }
 import Foundation
 struct Concentration
 {
 private(set) var cards = [Card]()
 private(set) var score = 0
 private(set) var flipCount = 0
 private var indexOfOneAndOnlyFaceUp: Int? {
 get {
 return cards.indices.filter({cards[0ドル].isFaceUp}).oneAndOnly
 }
 set {
 for flipDownIndex in cards.indices {
 cards[flipDownIndex].isFaceUp = (flipDownIndex == newValue)
 }
 }
 }
 private var selectedIndex = Set<Int>()
 private var lastIndexWasSelected = false
 /// returns true if all cards have been matched
 var allCardsHaveBeenMatched: Bool {
 for index in cards.indices {
 if !cards[index].isMatched { return false }
 }
 return true
 }
 /**
 Choose a card at an index.
 Handles flip count, if a card is faced up, and matching of cards
 */
 mutating func chooseCard(at index: Int){
 assert(cards.indices.contains(index), "Concentration.chooseCard(at:\(index)): index is not in the cards")
 let cardWasPreviouslySelected = selectedIndex.contains(index)
 if !cards[index].isMatched {
 // only flip cards that are visible
 flipCount += 1
 if let matchIndex = indexOfOneAndOnlyFaceUp, matchIndex != index {
 // 2 cards are face up, check if cards match
 if cards[index] == cards[matchIndex] {
 cards[index].isMatched = true
 cards[matchIndex].isMatched = true
 if lastIndexWasSelected {
 // add extra to account for subtracting earlier
 score += 3
 } else {
 score += 2
 }
 }else {
 // no match
 if cardWasPreviouslySelected {score -= 1}
 }
 cards[index].isFaceUp = true
 } else {
 // one card is selected, turn down other cards and set this card
 if cardWasPreviouslySelected { score -= 1 }
 indexOfOneAndOnlyFaceUp = index
 lastIndexWasSelected = cardWasPreviouslySelected
 }
 }
 selectedIndex.insert(index)
 }
 init(numberOfPairsOfCards: Int){
 assert(numberOfPairsOfCards > 0, "Concentraation.init(numberOfPairsOfCards:\(numberOfPairsOfCards) you must have multiple pairs of cards")
 for _ in 0..<numberOfPairsOfCards {
 let card = Card()
 cards += [card, card]
 }
 shuffleCards()
 }
 mutating private func shuffleCards() {
 for _ in 0..<cards.count {
 // sort seems better than .swap()
 cards.sort(by: {_,_ in arc4random() > arc4random()})
 }
 }
 }
 extension Collection {
 var oneAndOnly: Element? {
 return count == 1 ? first : nil
 }
 }
import Foundation
class Theme
{
 enum Theme: UInt32 {
 case halloween
 case love
 case animal
 case waterCreatures
 case plants
 case weather
 }
 /// get an array of icons by theme
 func getThemeIcons(by theme: Theme) -> [String] {
 switch theme {
 case .halloween:
 return ["👻", "😈", "👹", "👺", "🎃", "☠️", "🦇", "💀"]
 case .love:
 return ["💕","💋", "💌", "💘", "❤️", "💓", "💖", "💔"]
 case .animal:
 return ["🐶", "🐒", "🐔", "🦊", "🦉", "🐭", "🐥", "🦎"]
 case .waterCreatures:
 return ["🦑", "🐙", "🐠", "🐡", "🐳", "🐚", "🦀", "🦈"]
 case .plants:
 return ["🍁", "🍄", "🌹", "💐", "🌾", "🌸", "🌺", "🌻"]
 case .weather:
 return ["🔥", "❄️", "☀️", "💦", "☔️", "🌬", "☁️", "🌪", "⛈"]
 }
 }
 private func random() -> Theme {
 let max = Theme.weather.rawValue
 let randomIndex = arc4random_uniform(max + UInt32(1))
 return Theme(rawValue: randomIndex) ?? Theme.halloween
 }
 /**
 get a random array of themed icons
 - Author:
 Anna
 */
 func getRandomThemeIcons() ->[String] {
 return getThemeIcons(by: random())
 }
}
import UIKit
class ViewController: UIViewController {
 private lazy var game = Concentration(numberOfPairsOfCards: numberOfPairsOfCards)
 var numberOfPairsOfCards: Int {
 return (cardButtons.count + 1) / 2
 }
 @IBOutlet weak var finishedLabel: UILabel!
 @IBOutlet weak var scoreCountLabel: UILabel!
 @IBOutlet weak var flipCountLabel: UILabel!
 private var flipCount = 0 {
 didSet {
 flipCountLabel.text = "Flip Count: \(flipCount)"
 }
 }
 private var scoreCount = 0 { didSet { scoreCountLabel.text = "Score: \(scoreCount)"} }
 @IBOutlet var cardButtons: [UIButton]!
 @IBAction func touchCard(_ sender: UIButton) {
 if let cardNumber = cardButtons.index(of: sender){
 game.chooseCard(at: cardNumber)
 updateViewFromModel()
 }else {
 print("card is not in cardButton array")
 }
 }
 @IBAction func touchNewGame(_ sender: UIButton) {
 // reset game
 game = Concentration(numberOfPairsOfCards: (cardButtons.count + 1) / 2)
 // reset theme choices
 emojiChoices = theme.getRandomThemeIcons()
 // update view
 updateViewFromModel()
 }
 private func updateViewFromModel(){
 flipCount = game.flipCount
 scoreCount = game.score
 for index in cardButtons.indices {
 let button = cardButtons[index]
 let card = game.cards[index]
 if card.isFaceUp {
 button.setTitle(emoji(for: card), for: UIControlState.normal)
 button.backgroundColor = #colorLiteral(red: 1, green: 1, blue: 1, alpha: 1)
 } else {
 button.setTitle("", for: UIControlState.normal)
 button.backgroundColor = card.isMatched ? #colorLiteral(red: 1, green: 1, blue: 1, alpha: 0) : #colorLiteral(red: 1, green: 0.5763723254, blue: 0, alpha: 1)
 }
 }
 finishedLabel.textColor = game.allCardsHaveBeenMatched ? #colorLiteral(red: 1, green: 1, blue: 1, alpha: 1) : #colorLiteral(red: 0.9999960065, green: 1, blue: 1, alpha: 0)
 finishedLabel.text = game.score >= 0 ? "Nice work! 👍" : "Phew, 🐻ly made it"
 }
 private var theme = Theme()
 private lazy var emojiChoices = theme.getRandomThemeIcons()
 private var emoji = [Card: String]()
 private func emoji(for card: Card) -> String {
 if emoji[card] == nil, emojiChoices.count > 0 {
 emoji[card] = emojiChoices.remove(at: emojiChoices.count.arc4random)
 }
 return emoji[card] ?? "?"
 }
}
extension Int {
 var arc4random: Int {
 if self > 0 {
 return Int(arc4random_uniform(UInt32(self)))
 } else if self < 0 {
 return -Int(arc4random_uniform(UInt32(abs(self))))
 }else {
 return 0
 }
 }
}

• Turnipdabeets: why would this be down voted?? pretty unfriendly welcoming. :(
• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! Can you edit your post to include more details about this Memory game? If possible, please include the assignment/requirements.
• Turnipdabeets: @SamOnela I can link to a PDF but I have a feeling that won't fly here either. I was looking for best practices and general comments on my implementation because the app works. drive.google.com/drive/folders/…
• Dan Oberlam: Welcome to Code Review! Links can rot. Please include a description of the challenge here in your question.

Answer 1

Concentration.swift

And I have few notes:

1. It's better to describe what this entity is about. You can add comment above declaration.

import Foundation
struct Concentration
{
 private(set) var cards = [Card]()
 private(set) var score = 0

2. This code can be rewritеen:

var allCardsHaveBeenMatched: Bool {
 for index in cards.indices {
 if !cards[index].isMatched { return false }
 }
 return true
 }

better/shorter version:

var allCardsHaveBeenMatched: Bool { !cards.contains(where: { !0ドル.isMatched } }

3. This method is doing too much and it's hard to read it, consider to spit it into few smaller methods (Single Responsibility Principle).

/**
 Choose a card at an index.
 Handles flip count, if a card is faced up, and matching of cards
 */
mutating func chooseCard(at index: Int){

---

Card.swift

1. Use UUID() instead

private static var identifierFactory = 0
 private static func getUniqueIdentifer() -> Int {
 identifierFactory += 1
 return identifierFactory
 }

---

Theme.swift

Can be rewritten like this (Using enum without enclosing class):

import Foundation
enum Theme: UInt32 {
 case halloween
 case love
 case animal
 case waterCreatures
 case plants
 case weather
 /// get an array of icons by theme
 var icons: [String] {
 switch self {
 case .halloween:
 return ["👻", "😈", "👹", "👺", "🎃", "☠️", "🦇", "💀"]
 case .love:
 return ["💕","💋", "💌", "💘", "❤️", "💓", "💖", "💔"]
 case .animal:
 return ["🐶", "🐒", "🐔", "🦊", "🦉", "🐭", "🐥", "🦎"]
 case .waterCreatures:
 return ["🦑", "🐙", "🐠", "🐡", "🐳", "🐚", "🦀", "🦈"]
 case .plants:
 return ["🍁", "🍄", "🌹", "💐", "🌾", "🌸", "🌺", "🌻"]
 case .weather:
 return ["🔥", "❄️", "☀️", "💦", "☔️", "🌬", "☁️", "🌪", "⛈"]
 }
 }
 static func randomTheme() -> Theme {
 let max = Theme.weather.rawValue
 let randomIndex = arc4random_uniform(max + UInt32(1))
 return Theme(rawValue: randomIndex) ?? Theme.halloween
 }
}

To summarize: your code is not bad, but need some improvements. You are using encapsulation to hide implementation details and prefer values type (like structs) over reference types (classes). It's good stating point!

PS: Welcome to Code Review!!

• Martin R: Your suggestion for var allCardsHaveBeenMatched is shorter, but not better: It always traverses the entire array, even if a non-match has been found (it does not "short-circuit"). – Better use contains instead of reduce.
• Turnipdabeets: How does contains short-circuit?
• Andrew: It will return once first not matched card will be found