Simple War game in Swift

Question

I'm new to swift and I want to know if I'm doing something wrong or a harder way. I made a simple war game app. Tames of variables aren't correct, I know, but I was doing it quickly and I didn't worry about names of variables in that small project so I named them in English with Polish grammar.

import UIKit

class ViewController: UIViewController {

    @IBOutlet weak var LeftCardOutlet: UIImageView!
    @IBOutlet weak var RightcardOutlet: UIImageView!
    @IBOutlet weak var LeftScoreOutlet: UILabel!
    @IBOutlet weak var RightscoreOutlet: UILabel!

    var karty = ["card2","card3","card4","card5","card6","card7","card8","card9","card10","jack","queen","king"]

    var scoreplayer:Int=0
    var CPUscore:Int=0
    override func viewDidLoad() {
        super.viewDidLoad()

    }

    override func didReceiveMemoryWarning() {
        super.didReceiveMemoryWarning()

    }
    @IBAction func DealClicked(_ sender: UIButton) {
        var random1:Int = 0
        var random2:Int = 0
        random1 = Int(arc4random_uniform(UInt32(12)))
        random2 = Int(arc4random_uniform(UInt32(12)))
        var im1=UIImage(named:"\(karty[random1])")
        var im2=UIImage(named: "\(karty[random2])")
        LeftCardOutlet.image=im1
        RightcardOutlet.image=im2
        if random1>random2 {
            scoreplayer+=1
            LeftScoreOutlet.text=String(scoreplayer)
        }
        if random1<random2 {
            CPUscore+=1
            RightscoreOutlet.text=String(CPUscore)
        }
    }

}

Answer 1

Welcome to Swift! Everything looks like it's working correctly so I'm adding some thoughts that will apply as you continue working and as the code gets more complex...

You're not doing anything in your viewDidLoad or didReceiveMemoryWarning except call the super class so my preference is to remove them.

I'd really recommend not capitalizing your property and function names (e.g. use leftCardOutlet instead of LeftCardOutlet), once you start writing more in swift it will make reading your card harder.

I'd suggest extracting your random number generation into a computed property, and instead of hardcoding the number 12 using the size of your cards array.

Nothing wrong with using non-english grammar in variable names (actually Swift works really well with unicode so feel free to use fur Polish alphabet) but try to be consistent, e.g. either scorePlayer, scoreCPU or computerScore, playerScore -- not a mix of both.

Finally, to help prevent bugs/logic errors as the code gets more complex:

• Default to assigning new variables as 'let', and change only to var if you find you're modifying it at some later time.
• Don't create a variable if you're only going to use it in one place (e.g. next line or couple of lines down) shorter/more concise code is normally easier to read and rationalize as it gets more complex.
• When you have if/else logic make sure you're capturing all cases. In this case there's the chance the numbers will tie, it's worth adding a placeholder block with a brief comment there (or to print message) even if you've not decided yet what to do there.

A few minor changes:

import UIKit

class ViewController: UIViewController {

    // made outlet property names shorter, don't need to include that they are outlet in the name
    @IBOutlet weak var leftCard: UIImageView!
    @IBOutlet weak var rightCard: UIImageView!
    @IBOutlet weak var leftScore: UILabel!
    @IBOutlet weak var rightScore: UILabel!

    var karty = ["card2","card3","card4","card5","card6","card7","card8","card9","card10","jack","queen","king"]

    // (personal preference, but removed `Int` type declaration since it's easily implied)
    var scorePlayer = 0
    var scoreCPU = 0

    private var randomDeal: Int {
        // (instead of hardcoding 12, use the size of your cards array)
        return Int(arc4random_uniform(UInt32(karty.count)))
    }

    @IBAction func dealClicked(_ sender: UIButton) {

        // (use 'let' instead of 'var' since they won't change in the scopy of this function)
        let random1 = randomDeal
        let random2 = randomDeal

        // (got rid of `im1`, `im2` variables and just assigning directly here)
        leftCard.image = UIImage(named:"\(karty[random1])")
        rightCard.image = UIImage(named: "\(karty[random2])")

        if random1 > random2 {
            //player wins
            scorePlayer += 1
            leftScore.text = String(scorePlayer)
        }
        else if random1 < random2 {
            // computer wins
            scoreCPU += 1
            rightScore.text = String(scoreCPU)
        }
        else {
            // tie - do nothing
            print("tie: nobody wins")
        }
    }
}