1

I'm brand new to 'in depth' OOP and iOS development, so have been getting my hands dirty with it this week. I'm feeling more comfortable so decided to create a rock, paper, scissors game. I'm aware it's very simplistic and any critique on this VERY simple statement is welcome.

To compare the users and computer picks I created the compare picks function with a case statement embedded. The error i'm getting is that the function is not receiving anything to return, presumably because the return calls are all within the switch statement.

func comparePicks(var human: String, var computer: String) -> String {
    switch human{
        //Player Chooses Rock
        case "Rock":
            if computer == "Rock"{
                return "Draw"}
            else if computer == "Paper"{
                return "Lose"}
            else if computer == "Scissors"{
                return "Win"}
            //Player Chooses Scissors
        case "Scissors":
            if computer == "Scissors"{
                return "Draw"}
            else if computer == "Rock"{
                return "Lose"}
            else if computer == "Paper"{
                return "Win"}
            //Player Chooses Paper
        case "Paper":
            if computer == "Paper"{
                return "Draw"}
            else if computer == "Rock"{
                return "Lose"}
            else if computer == "Scissors"{
                return "Win"}
        default:
            return "error"
    }
    // Error here - "Missing return in a function expected to return 'String'
}

Where do these get returned too, can a switch statement have returns, I haven't seen anything mentioned in the docs. How can I change this so the return is being given to the compare Picks function instead. TIA.

  • 1
    The error you are seeing is because Swift isn't convinced you have covered all possibilities. Consider what happens if you pass in "Spock" for the computer. – vacawama May 31 '15 at 12:13
  • A nice way to solve your problem is to create a array 3x3 where you have 3 columns to indicate rock, scissors, paper and 3 rows to indicate the same and each intersection the value -1 lose, 0 draw, 1 win, so you can know the result just asking result = array[computerValue, userValue] – Icaro May 31 '15 at 12:19
  • Using an `enum` instead of a string as suggested below is probably the best solution here. In other situations you can use `fatalError()` to tell the compiler that some cases do "not occur", see for example http://stackoverflow.com/questions/30189505/missing-return-uitableviewcell. – Martin R May 31 '15 at 12:25

3 Answers3

2

You would be better served by using an enum instead of string literals for this. An enum accurately represents all of your allowable states and prevents you from making typo errors. And enum can also be more efficient than string comparisons.

In your code you just don't need the default: section of the switch because that is the fallback case and shouldn't be possible if there are no coding errors. So, remove that clause and move the return "error" to the last line of the function.

Wain
  • 118,658
  • 15
  • 128
  • 151
  • Your correct thanks, will change these to enums. I used strings originally as I knew how to pick a random string. I know the default: isn't needed, I had it in for testing purposes, it'll be removed after. – Dan Farrall May 31 '15 at 12:13
  • The default is needed if you are using strings. It won't be needed after you change to using enums. – vacawama May 31 '15 at 12:31
  • Definitely agree this would have been much easier. Although picking a random enum seems like the next challenge! – Dan Farrall May 31 '15 at 12:43
  • 1
    `enum Choices { case Rock, Scissors, Paper } let choice = [Choices.Rock, Choices.Scissors, Choices.Paper][Int(arc4random_uniform(3))]` – vacawama May 31 '15 at 12:48
  • Yes. That is tested. What problem are you seeing? Those are two lines of code, not one. – vacawama May 31 '15 at 13:57
  • It's just returning a nil value when called in a function. enum Choices { case Rock case Paper case Scissors } var computerPick: Choices! func opponentPick(){ let computerPick = [Choices.Rock, Choices.Scissors, Choices.Paper][Int(arc4random_uniform(3))] } opponentPick() println(computerPick) – Dan Farrall May 31 '15 at 15:11
  • I see your problem. Get rid of the `let` in `let computerPick = ...`. You are creating a new local variable called `computerPick` instead of changing the one you created before. Note, when you print the enum value you will just see "(Enum Value)" which isn't very useful. You can implement the `Printable` protocol like this: `enum Choices: Printable { case Rock, Paper, Scissors var description: String { switch self { case Rock: return "Rock" case Paper: return "Paper" case Scissors: return "Scissors" } } }` – vacawama May 31 '15 at 21:01
  • I added the enum solution to my answer because I had fun playing around with it. – vacawama May 31 '15 at 21:35
2

The returns in your switch statement do indeed leave the function, but you haven't accounted for all possible paths. If you call your function with "Spock" for the computer's choice, you won't hit any of your existing return statements.

If you replace the third comparison for the computer with an else, it works:

func comparePicks(var human: String, var computer: String) -> String {
    switch human{
        //Player Chooses Rock
    case "Rock":
        if computer == "Rock"{
            return "Draw"}
        else if computer == "Paper"{
            return "Lose"}
        else {
            return "Win"}

        //Player Chooses Scissors
    case "Scissors":
        if computer == "Scissors"{
            return "Draw"}
        else if computer == "Rock"{
            return "Lose"}
        else {
            return "Win"}

        //Player Chooses Paper
    case "Paper":
        if computer == "Paper"{
            return "Draw"}
        else if computer == "Rock"{
            return "Lose"}
        else {
            return "Win"}

    default:
        return "error"
    }

    // No error, all cases accounted for.
}

So if all of the paths are covered, you don't need an additional return statement. As @MartinR points out in the comments, this hides errors because "Spock" could now be a winning state when it should really be an error.

As other answers are suggesting, enums would be a better approach because then you avoid this possible error condition by having only 3 valid values for your enum.


Here is a solution using enums, just because I had fun playing with it...

enum Choices: String, Printable {
    case Rock = "Rock"
    case Paper = "Paper"
    case Scissors = "Scissors"

    var description: String {
        return self.rawValue
    }

    static func random() -> Choices {
        return [.Rock, .Paper, .Scissors][Int(arc4random_uniform(3))]
    }
}

func comparePicks(human: Choices, computer: Choices) -> String {
    switch human {

    //Player Chooses Rock
    case .Rock:
        switch computer {
        case .Rock:     return "Draw"
        case .Paper:    return "Lose"
        case .Scissors: return "Win"
        }

    //Player Chooses Scissors
    case .Scissors:
        switch computer {
        case .Scissors: return "Draw"
        case .Rock:     return "Lose"
        case .Paper:    return "Win"
        }

    //Player Chooses Paper
    case .Paper:
        switch computer {
        case .Paper:    return "Draw"
        case .Scissors: return "Lose"
        case .Rock:     return "Win"
        }
    }
}

let human = Choices.random()
println(human)  // prints "Paper"
let computer = Choices.random()
println(computer)  // Prints "Rock"
println(comparePicks(human, computer))  // Prints "Win"

By creating a tuple of the picks, you can combine the comparisons into one switch statement:

func comparePicks(human: Choices, computer: Choices) -> String {
    switch (human, computer) {
    case (.Rock, .Rock), (.Scissors, .Scissors), (.Paper, .Paper): return "Draw"
    case (.Rock, .Scissors), (.Scissors, .Paper), (.Paper, .Rock): return "Win"
    case (.Rock, .Paper), (.Paper, .Scissors), (.Scissors, .Rock): return "Lose"
    }
}
vacawama
  • 150,663
  • 30
  • 266
  • 294
  • But this approach "hides" programming errors, e.g. "SomeString" is treated the same way as "Scissors". – Martin R May 31 '15 at 12:33
  • Good point. I was trying to help them understand why the compiler was complaining and not necessarily suggesting that this is the way to code it. – vacawama May 31 '15 at 12:34
  • No it's a good answer, obviously i'm aware it should be done differently but good to know why also. – Dan Farrall May 31 '15 at 12:41
0

Your function must return an String, Swift compiler is not confident that this will always happen. I like to have one point of exit in my code (one return) right on the end and I use a variable to keep the result to be return, I believe it make easier to understand, but that is my personal choice. The solutions for you: You can add a question mark in the return so it does not have always to return something and you return nil in the end

func comparePicks(var human: String, var computer: String) -> String?{
    //some code
    return nil
}

Or you can just return a string that means something to you

func comparePicks(var human: String, var computer: String) -> String {
        //some code
        return "No Result"
}
Icaro
  • 14,585
  • 6
  • 60
  • 75