Problem
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)
}
}
}
Solution
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")
}
}
}