UPGRADE YOUR SKILLS: Learn advanced Swift and SwiftUI on Hacking with Swift+! >>

Code critique - Day 25 - Rock, Paper, Scissors

Forums > 100 Days of SwiftUI

I would be very appreciative if some more experienced developers could take a quick look at my code for this challenge and share some advice on how to improve it. While my app is working as intended, I feel like I could be doing something things more effciently when it comes to the actual logic of the program. Not so interested in the style stuff at the moment. Thanks in advance!

import SwiftUI

struct buttonLook: ViewModifier {
    func body(content: Content) -> some View {
        content
            .clipShape(RoundedRectangle(cornerRadius: 15.0))
            .shadow(color: .white, radius: 3, x: 0.0, y: 0.0)
    }
}

extension View {
    func buttonStyle() -> some View {
        self.modifier(buttonLook())
    }
}

struct ContentView: View {

    var possibleMoves = ["Rock", "Paper", "Scissors"]

    @State private var appChoice = Int.random(in: 0 ..< 3)
    @State private var playerWinLose = Bool.random()

    var winOrLoseText: String {
        if playerWinLose == true {
            return "WIN!"
        } else {
            return "LOSE!"
        }
    }

    @State private var playerScore = 0
    @State private var playCount = 1

    @State private var gameOver = false

    var body: some View {
        ZStack {

            LinearGradient(gradient: Gradient(colors: [Color.blue, Color(red: 0.2, green: 0.2, blue: 0.5)]), startPoint: .top, endPoint: .bottom).edgesIgnoringSafeArea(.all)

            VStack(spacing: 50) {

                Text("""
                    Exercise your brain muscle!
                    Collect points by winning or losing
                    based on the prompt below.
                    """).multilineTextAlignment(.center)

                VStack {
                        Text("Siri chooses")
                        Text("\(possibleMoves[appChoice])")
                            .font(.largeTitle)
                            .fontWeight(.bold)
                        Image(self.possibleMoves[appChoice])
                            .buttonStyle()
                    }

                VStack {
                    Text("Now, try to")
                    Text("\(winOrLoseText)")
                        .font(.largeTitle)
                        .fontWeight(.black)
                        .foregroundColor(playerWinLose ? Color.green : Color.red)

                }

                VStack(spacing: 5) {
                    HStack(spacing: 60) {
                        Text("Rock")
                        Text("Paper")
                        Text("Scissors")
                    }
                    HStack {
                        ForEach(0 ..< 3) { item in
                            Button(action: {
                                self.playerChoice(item)
                                continueGame()
                            }) {
                                Image(self.possibleMoves[item])
                                    .resizable()
                                    .frame(width: 100, height: 100)
                                    .buttonStyle()
                            }
                        }
                    }
                }

                VStack(spacing: 10) {
                    Text("Round \(playCount) of 10")
                        .fontWeight(.bold)
                    Text("Score: \(playerScore)")
                        .fontWeight(.bold)
                }
            }
            .foregroundColor(.white)
        }
        .alert(isPresented: $gameOver) {
            Alert(title: Text("Game over!"), message: Text("Your final score was \(playerScore)/10"), dismissButton: .default(Text("Play again")) {
                resetGame()
            })
        }
    }

    func playerChoice(_ item: Int) {
        //If player should try and WIN:
        if playerWinLose == true {
            //All the losing scenarios
            if item == appChoice {
                print("Lose - Items the same")
            } else if item == 2 && appChoice == 0 {
                print("Lose - Player chose Scissors, Siri chose Rock")
            } else if item == 0 && appChoice == 1 {
                print("Lose - Player chose Rock, Siri chose Paper")
            } else if item == 1 && appChoice == 2 {
                print("Lose - Player chose Paper, Siri chose Scissors")
            } else {
                print("Win! Add Points")
                playerScore += 1
            }
        } else {
            //If player should try and LOSE:
                //All the winning scenarios (because copy/paste from above was easy xD)
            if item == appChoice {
                print("Win - Items the same")
                playerScore += 1
            } else if item == 2 && appChoice == 0 {
                print("Win - Player chose Scissors, Siri chose Rock")
                playerScore += 1
            } else if item == 0 && appChoice == 1 {
                print("Win - Player chose Rock, Siri chose Paper")
                playerScore += 1
            } else if item == 1 && appChoice == 2 {
                print("Win - Player chose Paper, Siri chose Scissors")
                playerScore += 1
            } else {
                print("Lose - No Points")
            }
        }
    }

    func continueGame() {
        if playCount == 10 {
            gameOver = true
        } else {
            playCount += 1
            appChoice = Int.random(in: 0 ..< 3)
            playerWinLose = Bool.random()
        }
    }

    func resetGame() {
        appChoice = Int.random(in: 0 ..< 3)
        playerWinLose = Bool.random()
        playCount = 1
        playerScore = 0
    }
}

struct ContentView_Previews: PreviewProvider {
    static var previews: some View {
        ContentView()
    }
}

3      

I would work mainly on naming. playerWinLose - without reading all the usages it is hard to know what this variable means. Something like playerShouldWin would be more readable. Or you can create two case enum, something like PlayerGoal with cases win and lose.

} else if item == 2 && appChoice == 0 {
                print("Win - Player chose Scissors, Siri chose Rock")
                playerScore += 1
            } else if item == 0 && appChoice == 1 {
                print("Win - Player chose Rock, Siri chose Paper")
                playerScore += 1
            } else if item == 1 && appChoice == 2 {
                print("Win - Player chose Paper, Siri chose Scissors")
                playerScore += 1
            } else {

This could be single line, you already have an array possibleMoves which should also be let btw and you can get the choices each player played with indexes.

6      

Thanks for taking the time to look at it and thank you for the advice!

3      

Hacking with Swift is sponsored by RevenueCat

SPONSORED Take the pain out of configuring and testing your paywalls. RevenueCat's Paywalls allow you to remotely configure your entire paywall view without any code changes or app updates.

Learn more here

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

Howdy,

I am a novice, as in I just completed the Day 25 Challenge yesterday, so we're in the same boat.

Congratz!

I was able to follow your code very easily & you did a few things I did not such as using a struct to format the look of your buttons & implementing an extension. Not a negative, just pointing out some differences in how we solved the same project & kind of bummed I didn't create a struc to do something in my code...maybe next project.

It's hard for me to say anything in particular given my lack of xp & in building my own project I specifically didn't use a few techniques, because they were ones I just used making Guess the Flag & maybe you took this concept to mind in making Rock. Paper. Scissors.

The only thing I notice is the use of your nestedIf...then...else statements. I 'believe' Paul suggested using a switch when we end up in situations like this to make it easier to read & follow the logic...but GREAT JOB!

I do all of my work/study with #100DaysOfSwiftUI in my stream, so I know it took me ~17 hours over 5ish days to complete this project!

Baron Blackbird

3      

To start, I have been on the books until recently, so I can't really comment on some stuff here since I haven't done the 100 days, but I will add to the above comments with more details that I learned from the books. So for what it's worth, here goes:

1- since possibleMoves won't change it should be let. I'm fairly certain Xcode suggested that.

2- Because playerWinLose is a boolean, you don't need to check if playerWinLose == true, but just say if playerWinLose {...}. This is where the suggestion to rename it to playerShouldWin comes in handy, since your code becomes for example:

        if playerShouldWin {
            return "WIN!"
        } else {
            return "LOSE!"
        }

This reads much better as mentioned above.

3- Considering how many conditions you check, I agree with the above comment that a switch would make things much clearer. But you might not lke the way it looks. You would have to switch over item and appChoice as a tuple:

switch (item, appChoice) {
case (0, 0), (1, 1), (2, 2):
    print("Lose - Items the same")
case (2, 0):
    print("Lose - Player chose Scissors, Siri chose Rock")
case (0, 1):
    print("Lose - Player chose Rock, Siri chose Paper")
case (1, 2):
    print("Lose - Player chose Paper, Siri chose Scissors")
default:
    print("Win! Add Points")
    playerScore += 1
}

Whether that is something you like or not is up to you. They both work almost the same. Except a switch statement ensures that you cover all the possibilities (being exhaustive), hence the default case which covers all others.

4- Finally, regardless of which you end up choosing, if-else or switch, you can use string interpolation for your print statements without writing out what each side chose:

print("Lose - Player chose \(possibleMoves[item]), Siri chose \(possibleMoves[appChoice])")

This will automatically print the correct choices.

Hope that helps!

Keep it up mate. Hope you're enjoying it. Paul certainly has a fun way of explaining things. All the best.

3      

I have gone and read the Day 25 details, and gave it some thoughts. First though, you should know your app logic is sound. Also, in the text you will see he actually writes if shouldWin, so my clarification above is correct. (it should be, after all I learned it from him).

I did not wish to edit the above comment, so I will simpy add a simplification of the switch statement here, which makes the string interpolation more relevant:

switch (item, appChoice) {
case (0, 0), (0, 1), (1, 1), (1, 2), (2, 0), (2, 2):
    print("Lose - Player chose \(possibleMoves[item]), Siri chose \(possibleMoves[appChoice])")
default:
    print("Win! Add Points")
    playerScore += 1
}

This is why in his "hints" he suggests simplifying the logic of how you determine the result. Essentially you are iterating through all possible scenarios here, but there is another way to look at it as he suggests.

It has to do with the relationship between the moves.

5      

@mstbr  

Hey everyone, hey @MarcusKay:

It has to do with the relationship between the moves.

I believe I'm trying to tackle what you and Paul are alluding to: Given an ordering of the array of possible moves that corresponds to the actual rules of the game (and to its name ๐Ÿ™‚), the "winning" countermove would be "on the right" of the move (+1), with the "losing" countermove "on its left" (-1). Now, while I like how the below code reads, it only works if the logic doesn't have to cross the boundaries of the array, because the resulting subscripts will be out of bounds and the thing crashes.

I can't for the life of me figure out how to accomplish the necessary wrap-around behaviour. Any help would be greatly appreciated!


// Variables:
    @State private var score = 0
    @State private var move = Int.random(in: 0...2)
    @State private var playerShouldWin: Bool = Bool.random()

    let moves = ["Rock", "Paper", "Scissors"]

// Button function(s):
func playerChooses(_ chosenMove: String) {
        if playerShouldWin == true && chosenMove == moves[move + 1] {
            score += 10
            nextTurn()
        } else if playerShouldWin == false && chosenMove == moves[move - 1] {
            score += 10
            nextTurn()
        } else {
            score -= 10
            nextTurn()
        }
    }

    func nextTurn() {
        move = Int.random(in: 0...2)
        playerShouldWin = Bool.random()
    }
}

3      

Well I'm not sure if I have the most elegant solution, but what is the relationship between rock and scissors? it's the index of scissors - 2...

Also, a tip on the if statement. It basically checks this:

if (whatever is here is true) { ... }

A boolean is either True or False. So you don't need to use == So you could change your if statement to:

if playerShouldWin && (chosenMove == moves[move + 1] || chosenMove == moves[move - 2] ) {
  //congrats player chose correctly
} else if !playerShouldWin && (chosenMove == moves[move - 1] || chosenMove == moves[move + 2]) {
  //congrats
} else {
  // too bad
}

That was my approach. I didn't overthink it to be honest. There might be some better way, but for this stage, I think we're good ๐Ÿ˜‰

4      

@mstbr  

@MarcusKay:

what is the relationship between rock and scissors? it's the index of scissors - 2...

Thanks so much for your answer! To my eyes, it made so much sense immediately (I had definitely been overthinking things). However, my code still crashes in (I believe) the 'wrap-around' cases. Do you have any idea why?

P.S. Also thanks on the pointer re: booleans. Makes for much more naturally-reading code!

struct ContentView: View {

    @State private var score = 0
    @State private var move = Int.random(in: 0...2)
    @State private var playerShouldWin: Bool = Bool.random()
    @State private var turnCount = 0
    @State private var showScore = false

    let moves = ["Rock", "Paper", "Scissors"]

    var body: some View {
        ZStack {
            LinearGradient(gradient: Gradient(colors: [.yellow, .green]), startPoint: .center, endPoint: .bottom)
            VStack(spacing: 30) {
                Spacer()
                Text("Computer chose:")
                Text("\(moves[move])")
                HStack {
                    Text("You should")
                    Text("\(playerShouldWin ? "win" : "lose")")
                        .fontWeight(.bold)
                        .foregroundColor(playerShouldWin ? .green : .red)
                    Text("this turn.")
                }
                Text("Your choice:")
                HStack(spacing: 30) {
                    ForEach(moves, id: \.self) { playerMove in
                        Button(action: {
                            playerChooses(playerMove)
                        }) {
                            Text(playerMove)
                                .fontWeight(.bold)
                        }
                    }
                }
                Spacer()
            }
            .font(.largeTitle)
            .padding()
        }
        .edgesIgnoringSafeArea(.all)
        .alert(isPresented: $showScore) {
            Alert(title: Text("Your score is:"), message: Text("\(score)"), dismissButton: .default(Text("Continue")) {
                self.endGame()
            })
        }
    }

    func endGame() {
        score = 0
        turnCount = 0
        showScore = false
        move = Int.random(in: 0...2)
        playerShouldWin = Bool.random()
    }

    func nextTurn() {
        turnCount += 1
        if turnCount >= 5 {
            showScore = true
        }
        move = Int.random(in: 0...2)
        playerShouldWin = Bool.random()
    }

    func playerChooses(_ chosenMove: String) {
        if playerShouldWin && (chosenMove == moves[move + 1] || chosenMove == moves[move - 2]) {
            score += 10
            nextTurn()
        } else if !playerShouldWin && (chosenMove == moves[move - 1] || chosenMove == moves[move + 2]) {
            score += 10
            nextTurn()
        } else {
            score -= 10
            nextTurn()
        }
    }

    struct ContentView_Previews: PreviewProvider {
        static var previews: some View {
            ContentView()
        }
    }
}

3      

Woops ๐Ÿ˜Š I was working on another version for another thread and completely did not notice the array. The thing is if the player should win, and the app chose paper (meaning 1) and the user chooses anything other than scissors (2) then we will get an out of index error. Because as soon as it hits the move - 2 part, it will become -1 which is wrong... similar issue with the others.

The quick fix is to convert your player's move into an int, and compare those.

    func playerChooses(_ chosenMove: String) {
        guard let playerChoice = moves.firstIndex(of: chosenMove) else { return }
        if playerShouldWin && (playerChoice == move + 1 || playerChoice == move - 2) {
            score += 10
            nextTurn()
        } else if !playerShouldWin && (playerChoice == move - 1 || playerChoice == move + 2) {
            score += 10
            nextTurn()
        } else {
            score -= 10
            nextTurn()
        }
    }

Sorry about that... I actually tested that... and it works... unless I missed something... in which case, I will need to take a break... what day is it? ๐Ÿ˜œ

4      

@mstbr  

Yep, that does it! I feel like it shouldn't have been such a braintwister for me though, so I guess it's time to take a break and stop looking at the screen for a bit ๐Ÿ˜ƒ

Thank you for your kind help.

4      

Hacking with Swift is sponsored by RevenueCat

SPONSORED Take the pain out of configuring and testing your paywalls. RevenueCat's Paywalls allow you to remotely configure your entire paywall view without any code changes or app updates.

Learn more here

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.

 
Unknown user

You are not logged in

Log in or create account
 

Link copied to your pasteboard.