## Day 25 Solution - Constructive Criticism Requested

Hello,

I'm looking for some constructive criticism please. Although I added a little on my own this time for fun, my code always seems more verbose than others. Would love some thoughts on how to make things more efficient or the general idea that I am ok with how things are being done.

``````struct ContentView: View {

// MARK: - CONSTANTS
let choices: [String] = ["πͺ¨", "π", "βοΈ", "π¦", "ππ½", "πͺ¨", "π", "βοΈ", "π¦", "ππ½"]
let winMoves: [String] = ["π", "βοΈ", "ππ½", "πͺ¨", "π¦", "ππ½", "π¦", "πͺ¨", "βοΈ", "π"]
let loseMoves: [String] = ["π¦", "πͺ¨", "π", "ππ½", "βοΈ", "βοΈ", "ππ½", "π¦", "π", "πͺ¨"]

// MARK: - @STATE PROPERTIES
@State private var appChoice: Int = 0                       // Randomize this in the setCriteria function randomly
@State private var appShouldWin: Bool = true                // Toggle this in the setCriteria function randomly
@State private var correctAnswers: [String] = []
@State private var userChoice: Int = 0
@State private var attemptNumber: Int = 1
@State private var score: Int = 0
@State private var answerEvaluation: String = "Right"
@State private var gameReset: Bool = true

// MARK: - ALERT PROPERTIES
@State private var showingAlert: Bool = false
@State private var isEndGame: Bool = false

// MARK: - FUNCTIONS
func setCriteria() {
if gameReset {
appShouldWin = Bool.random()
attemptNumber = 1
score = 0
}
appShouldWin.toggle()
appChoice = Int.random(in: 0...4)

if appShouldWin {
} else {
}
}

func evaluateAnswer(choice: Int) {
score += 1
} else {
score -= 1
}
attemptNumber += 1
if attemptNumber > 10 {
isEndGame = true
gameReset = true
} else {
gameReset = false
}
}

// MARK: - BODY
var body: some View {
ZStack {
Color.blue
.ignoresSafeArea()

VStack(spacing: 30) {
Spacer()

Text("Rock, Paper, Scissors,\nLizard, Spock")
.font(.system(size: 40))
.multilineTextAlignment(.center)

Group {
Divider()

Text("""
Scissors cuts Paper, Paper covers Rock,
Rock crushes Lizard, Lizard poisons Spock,
Spock smashes Scissors,
Scissors decapitate Lizard,
Lizard eats Paper, Paper disproves Spock,
Spock vaporizes Rock, and as it always has...
Rock crushes scissors!
""")
.font(.system(size: 18))
.multilineTextAlignment(.center)

Divider()
}

Group {
HStack {
Text("The app chose:")
Text(choices[appChoice])
.font(.title)
}
VStack(spacing: 0) {
HStack {
Text("If the player should")
Text(appShouldWin ? "WIN" : "LOSE")
.foregroundColor(appShouldWin ? Color.green : Color.red)
.fontWeight(.bold)
Text("...")
}
Text("Which is the right choice?")
}
}

HStack(spacing: 10) {
ForEach(0..<5) { number in
Button {
userChoice = number
} label: {
Text(choices[number])
.font(.system(size: 40))
}
.buttonStyle(.borderedProminent)
}
} //: Button HStack
Button("Continue", action: setCriteria)
} message: {
Text("You chose \(choices[userChoice])")
}
.alert("End Of Game", isPresented: \$isEndGame) {
Button("Reset", action: setCriteria)
} message: {
Text("You scored \(score) out of 10!")
}

Group {
Spacer()

Text("Current Score: \(score)")
}
} //: Outer VStack
.foregroundColor(.white)
.font(.system(size: 24))
} //: ZStack
.onAppear() {
setCriteria()
}
} //: Body
} //: ContentView``````

2

Michael observes:

My code always seems more verbose than others. Would love some thoughts on how to make things more efficient.

Your code is verbose because your logic is verbose! A cursory look at your solution leads me to think that you have a background in procedural languages (VisualBasic, C++, JAVA, etc) and haven't quite made the jump to SwiftUI's declarative style.

The good news is that you're learning! The better news is as you gain experience, you'll start to see some inefficiencies and your code will get better and less verbose. It will become more Swifty.

Well you asked for thoughts, and not a lecture. So here are a few to get you started.

#### MARK

Ten points to Ravenclaw for using MARK tags in your code!

#### ContentView

Minus ten points from Ravenclaw for naming your view `ContentView`. Ugh. Try `GameView`, or something more descriptive!

#### Verbose Declarations

You declare your structs variables like this:

``````    // Not very Swifty. And very verbose.
@State private var appChoice: Int        = 0      // Compiler knows zero is an Int
@State private var appShouldWin: Bool    = true   // Compiler knows true is a Bool
@State private var userChoice: Int       = 0      // etc``````

Moving forward, trust the force compiler!

``````    // Less verbose! More Swifty.
@State private var appChoice    = 0      // Compiler knows zero is an Int
@State private var appShouldWin = true   // Compiler knows true is a Bool
@State private var userChoice   = 0      // etc``````

#### Calculated Properties

You'll start to learn the benefits of calculated properties as you progress. This is a great way to consolidate your logic while reducing code. Take this use case for example: When do you want to end a game?

You have this code:

`` @State private var gameReset: Bool = true``

This is referenced in several places in your code, tied to some value (10). It has to be set and reset. This is verbose, and can lead to logic errors trying to keep these syncronized.

The Swift way is to declare it!

`````` @State private var gameLength  = 5       // Only go five rounds, then the game ends.
@State private var gameReset   = false   // Yikes! Now you have to keep this syncronized with gameLength.``````

Instead of scattering logic throughout your code, consider the benefit of asking Swift to calculate if your player is at the end of a game.

`````` @State private var gameLength  = 5           // Only go five rounds, then the game ends. Change this in ONE place
private var gameReset: Bool { attemptNumber > gameLength }  // Game resetting logic in ONE place.``````

This allows you to simplify your logic in your `evaluateAnswer` function.

``````// Nice! Declare what you want to do. Push your game logic to ONE place.
// Resetting logic doesn't belong here.
if gameReset {  // can easily change the gameLength, for testing, or in User Preferences
isEndGame = true
// gameReset = true   // you don't need to hand cram your logic here.
} else {
// gameReset = false // you don't need to hand cram your logic here.
}``````

#### Declarative Code

Nota Bene: It is no secret that SwiftUI's declarative style is copied from many other languages. Hush now.

2

Phenomenal answer. This is exactly what I was looking forward and very much appreciated.

I did move to Swift/SwiftUI from VB and C++ (a little bit of HTML and Java) and am really enjoying this learning experience.

Thank you again for the review. I will look to use this for future lessons/projects.

2

Hi `@Icemonster13`,

I agree with `@Obelix` remarks about added `Type` as this can be inferred by the complier but there will be times when you will have to eg `var somePoint: CGFloat = 0` (however if you want to then you can but think you find that most people do not).

Also going on general put

1. Property
2. Computed Property (with `var body: some View` first)
3. Methods (`func`)

as this is the `View` as the main code

With that it a bit big so you could factor it out. I started with the first bit of the `View`. Made a SwiftUI file called `HeaderView` and put this in. (See `ContentView` at end to see it used). Yes still used `ContentView` as I think of this as the first view in a project if you rename it then you will always have to

``````struct HeaderView: View {
var body: some View {
VStack(spacing: 30) {
Text("Rock, Paper, Scissors, Lizard, Spock")
.font(.largeTitle)
.multilineTextAlignment(.center)

Divider()

Text("""
Scissors cuts Paper, Paper covers Rock,
Rock crushes Lizard, Lizard poisons Spock,
Spock smashes Scissors,
Scissors decapitate Lizard,
Lizard eats Paper, Paper disproves Spock,
Spock vaporizes Rock, and as it always has...
Rock crushes scissors!
""")
.multilineTextAlignment(.center)
.font(.body)

Divider()
}
}
}``````

NOTE change your Fonts size to use general one as these will size up and down depanding on thier size perference. Next look at the the next section. (you put them in `Group` in your code) Made a SwiftUI file callled `GameView`. This is little bit more complicated as has elements that change so we need to pass in those.

``````struct GameView: View {
let chosen: String
let appShouldWin: Bool

var body: some View {
VStack {
HStack {
Text("The app chose:")
Text(chosen)
.font(.title)
}

HStack(spacing: 5) {
Text("If the player should")
Text("\(appShouldWin ? "WIN" : "LOSE")β¦")
.foregroundColor(appShouldWin ? Color.green : Color.red)
.fontWeight(.bold)
}

Text("Which is the right choice?")
}
}
}``````

Next looked at `.alert` as you have two so with a bit of change around was able to use one with these properties.

``````// ALERT PROPERTIES
@Published var showingAlert = false
@Published var alertTitle = ""
@Published var alertMessage = ""
@Published var alertButton = ""
@Published var alertAction: (() -> Void)?``````

Also look at the the starting properties and your put `@State private var appChoice: Int = 0` when you can put

``````@Published var appChoice = Int.random(in: 0...4)
@Published var appShouldWin = Bool.random()
@Published var correctAnswers = [String]()``````

(I have used `@Published` but in the `View` use `@State private`, will explain why!). Now look at the methods So change the `setCriteria` to this (it took me some time to get why you put in array two answer but then worked out you can get there are two solution to each one).

``````func continueGame() {
appShouldWin = Bool.random()
appChoice = Int.random(in: 0...4)

if appShouldWin {
} else {
}
}``````

``````func resetGame() {
score = 0
attemptNumber = 1

continueGame()
}``````

and change your `evaluateAnswer` to this. As you see this is where the alert text comes in and the `alertAction` will be set.

`````` func evaluateAnswer(choice: Int) {
attemptNumber += 1

if attemptNumber > 10 {
alertTitle = "End Of Game"
alertMessage = "Your score is \(score) out of 10!"
} else {
score += 1
} else {
score -= 1
}

alertMessage = "You have chosen \(choices[choice])"
}

}``````

Lastly moved to ViewModel, so created a Swift file called `ContentViewModel` and put this in

``````extension ContentView {
class ViewModel: ObservableObject {
let choices = ["πͺ¨", "π", "βοΈ", "π¦", "ππ½", "πͺ¨", "π", "βοΈ", "π¦", "ππ½"]
let winMoves = ["π", "βοΈ", "ππ½", "πͺ¨", "π¦", "ππ½", "π¦", "πͺ¨", "βοΈ", "π"]
let loseMoves = ["π¦", "πͺ¨", "π", "ππ½", "βοΈ", "βοΈ", "ππ½", "π¦", "π", "πͺ¨"]

@Published var appChoice = Int.random(in: 0...4)
@Published var appShouldWin = Bool.random()
@Published var correctAnswers = [String]()

@Published var attemptNumber = 1
@Published var score = 0

@Published var showingAlert = false
@Published var alertTitle = ""
@Published var alertMessage = ""
@Published var alertButton = ""
@Published var alertAction: (() -> Void)?

func evaluateAnswer(choice: Int) {
attemptNumber += 1

if attemptNumber > 10 {
alertTitle = "End Of Game"
alertMessage = "Your score is \(score) out of 10!"
} else {
score += 1
} else {
score -= 1
}

alertMessage = "You have chosen \(choices[choice])"
}

}

func continueGame() {
appShouldWin = Bool.random()
appChoice = Int.random(in: 0...4)

if appShouldWin {
} else {
}
}

func resetGame() {
score = 0
attemptNumber = 1

continueGame()
}
}
}``````

So now the `ContentView` has now gone from over 150 lines to just over 50 lines with only the `View`

``````struct ContentView: View {
@StateObject private var vm = ViewModel()

var body: some View {
ZStack {
Color.blue
.ignoresSafeArea()

VStack {

GameView(chosen: vm.choices[vm.appChoice], appShouldWin: vm.appShouldWin)

HStack(spacing: 10) {
ForEach(0..<5) { number in
Button {
} label: {
Text(vm.choices[number])
.font(.system(size: 40))
}
.buttonStyle(.borderedProminent)
.tint(.cyan)
}
}
} message: {
}

Group {
Spacer()

Text("Current Score: \(vm.score)")
}
}
.foregroundColor(.white)
.font(.title)
.onAppear(perform: vm.continueGame)
}
}
}``````

PS Added `.tint` to Button to make it stand out from the background a bit.

2

Thanks @NigelGee. Excellent information and much appreciated.

Admittedly, some of your answer is a beyond my current skill-set (Day 25) but I fully understand the concepts you are showing and feel good about that.

This will really help me moving forward in the 100 Day course.

Thanks again!

2

Glad you appreciated the answer, when I started I was only going to suggest to refactor the `body` by making new SwiftUI files, but as I was getting to know your app thought of slightly better way to do the logic eg getting rid of the `endGame` boolean etc. Sorry got carried away, but it was fun and hope you havee fun in your coding.

2

SPONSORED Superwall lets you build & test paywalls without shipping updates. Run experiments, offer sales, segment users, update locked features and more at the click of button. Best part? It's FREE for up to 250 conversions / mo and the Superwall team builds out 100% custom paywalls β free of charge.

Sponsor Hacking with Swift and reach the world's largest Swift community!

### Archived topic

This topic has been closed due to inactivity, so you can't reply. Please create a new topic if you need to.

All interactions here are governed by our code of conduct.

You are not logged in