4
\$\begingroup\$

My code actually works. I was just wondering if there was alternative ways, maybe even more efficient ways of coding my program. This is suppose to be a game of drawing stick, and it's recording the score after each game. If I'm missing anything can someone let me know? Sort of new at this. Here is my code:

import UIKit
import QuartzCore

class PlayerVsPlayerViewController: UIViewController {

    var stats = GameStats.sharedStats
    var isPlayingAi = false
    var ai: SticksAi?

    @IBOutlet weak var Player2Score: UILabel!
    @IBOutlet weak var Player1Score: UILabel!
    @IBOutlet weak var SticksLeft: UILabel!
    @IBOutlet weak var SticksSlider: UISlider!
    @IBOutlet weak var PlayerWinsLabel: UILabel!
    @IBOutlet weak var playerChoiceSticksLabel: UILabel!
    @IBOutlet weak var btnStartGame: UIButton!
    @IBOutlet weak var NumberOfSticksSegmentCtrl: UISegmentedControl!
    @IBOutlet weak var btnStartNewGame: UIButton!
    @IBOutlet weak var lblAiChoice: UILabel!

    var currentSticks = 0
    var sticksTaken = 0
    var p1Score = 0
    var p2Score = 0
    var whoWins = ""
    var isPlayerNumber = 1
    var originalSticksCount = 0

    override func viewDidLoad() {
        super.viewDidLoad()
        lblAiChoice.isHidden = true
        PlayerWinsLabel.text = whoWins
        btnStartNewGame.isHidden = true
        SticksLeft.text = String(55)
        Player1Score.text = String(p1Score)
        Player2Score.text = String(p2Score)
        SticksSlider.value = 55
        NumberOfSticksSegmentCtrl.isEnabled = false
        print("Am i playing against an AI: \(isPlayingAi)")

        if ai == nil {
            ai = SticksAi(withStickCount: currentSticks)
        }
    }

    @IBAction func sliderMoved(_ slider: UISlider) {

        let value = lroundf(slider.value)
        currentSticks = value
        originalSticksCount = value
        if isPlayingAi {
            ai = SticksAi(withStickCount: currentSticks)
        }

        SticksLeft.text = String(currentSticks)

    }

    @IBAction func segmentSlected(_ segment: UISegmentedControl) {
            currentSticks -= segment.selectedSegmentIndex + 1
            segment.setEnabled(currentSticks >= 3, forSegmentAt: 2)
            segment.setEnabled(currentSticks >= 2, forSegmentAt: 1)
            SticksLeft.text = String(currentSticks)
            segment.selectedSegmentIndex = -1
            isPlayerNumber = isPlayerNumber == 1 ? 2 : 1
            if isPlayerNumber == 1 {
                playerChoiceSticksLabel.text = "Player 1: choose your sticks"
            } else {
                if isPlayingAi {
                    aiTurn()
                    if currentSticks > 0 {
                       isPlayerNumber = 1
                    }
                } else {
                    playerChoiceSticksLabel.text = "Player 2: choose your sticks"
                }
            }
            if currentSticks == 0 {
                NumberOfSticksSegmentCtrl.isEnabled = false
                if isPlayingAi && isPlayerNumber == 2 {
                    PlayerWinsLabel.text = "AI Wins!"
                } else {
                    PlayerWinsLabel.text = "Player \(isPlayerNumber) Wins!"
                }
                if isPlayerNumber == 1 {
                    p1Score += 1
                    stats.addGame(type: isPlayingAi ? .PlayerVsAi : .PlayerVsPlayer, won: true, sticks: originalSticksCount, playerMoves: 0, opponentMoves: 0)
                } else {
                    p2Score += 1
                    stats.addGame(type: isPlayingAi ? .PlayerVsAi : .PlayerVsPlayer, won: false, sticks: originalSticksCount, playerMoves: 0, opponentMoves: 0)
                }
                Player1Score.text = String(p1Score)
                Player2Score.text = String(p2Score)
                playerChoiceSticksLabel.isHidden = true
        }
    }

    @IBAction func startGame() {
        lblAiChoice.isHidden = true
        NumberOfSticksSegmentCtrl.isEnabled = true
        SticksSlider.isEnabled = false
        btnStartGame.isHidden = true
        btnStartNewGame.isHidden = false
        NumberOfSticksSegmentCtrl.setEnabled(true, forSegmentAt: 2)
        NumberOfSticksSegmentCtrl.setEnabled(true, forSegmentAt: 1)
        isPlayerNumber = 1
        playerChoiceSticksLabel.isHidden = false
    }

    @IBAction func startNewGame() {
        lblAiChoice.isHidden = true
        btnStartNewGame.isHidden = true
        btnStartGame.isHidden = false
        SticksSlider.isEnabled = true
        NumberOfSticksSegmentCtrl.isEnabled = false
        SticksLeft.text = String(55)
        Player1Score.text = String(p1Score)
        Player2Score.text = String(p2Score)
        SticksSlider.value = 55
        currentSticks = Int(SticksLeft.text!)!
    }

    func aiTurn() {
        if isPlayerNumber == 2 && currentSticks > 0 {

            let aiMove = ai?.takeTurn(sticksLeft: currentSticks)
            currentSticks -= aiMove!
            SticksLeft.text = String(currentSticks)
            lblAiChoice.isHidden = false
            lblAiChoice.text = "AI has chosen \(aiMove!) sticks."
        }
    }



    override func didReceiveMemoryWarning() {
        super.didReceiveMemoryWarning()
        // Dispose of any resources that can be recreated.
    }

    /*
    // MARK: - Navigation

    // In a storyboard-based application, you will often want to do a little preparation before navigation
    override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
        // Get the new view controller using segue.destinationViewController.
        // Pass the selected object to the new view controller.
    }
    */

    @IBAction func closeGame(_ sender: UIButton) {
        dismiss(animated: true, completion: nil)
    }
}
\$\endgroup\$
  • \$\begingroup\$ please: alsways lowercase the first letter if it is an instance of an class or an element. this helps do distinct it from a class and if you call a class method \$\endgroup\$ – muescha Apr 21 '17 at 22:08
1
\$\begingroup\$

As @muescha mentions in the comment, variables should be lowercased. Here are a few other quick improvements I would suggest in the current code: the naming of the variable isPlayerNumber is rather misleading as conventions indicates this to be a Bool while it is in reality an Int. I've changed it to simply read playerNumber below:

I would also suggest simplifying the "choose your sticks" part of your segmentSelected function:

if isPlayingAi {
    aiTurn()
    if currentSticks > 0 {
        playerNumber = 1
    }
} else {
    playerChoiceSticksLabel.text = "Player \(playerNumber): choose your sticks"
}

There is some code duplication that can be avoided further down in the same function:

if isPlayerNumber == 1 {
    p1Score += 1
} else {
    p2Score += 1
}
stats.addGame(type: isPlayingAi ? .PlayerVsAi : .PlayerVsPlayer, won: false, sticks: originalSticksCount, playerMoves: 0, opponentMoves: 0)

I think I would rejig the sticksLeft-label and currentSticks logic as well. (Perhaps renaming the latter stickCount at the same time). Then rather than setting the sticksLeft.text = "" everywhere you could do:

var stickCount: Int {
    didSet {
        sticksLeft.text = "\(stickCount)"
    }
}

in its current scope I think you solution is generally fine, but if you plan to expand on this it might make sense to separate things further, perhaps having an array of Player structs rather than p1Score, p2Score, isPlayingAi etc. as well.

\$\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.