is there a way to make this if statement shorter? or easier to maintain?

Question

I made a (rock,paper,scissors) app for education, is there a way to make this if statement shorter?

This is the swift code snippet :

var playerSelection = "" // possible values are r,p,s
var cpuSelection    = "" // possible values are r,p,s
var resultString    = ""

// DECIDE PLAYER RESULT
if(playerSelection == "r")
{
    if(cpuSelection == "r")
    {
        resultString = "draw"
    }
    else if(cpuSelection == "p")
    {
        resultString = "lose"
    }
    else if(cpuSelection == "s")
    {
        resultString = "win"
    }
}
else if(playerSelection == "p")
{
    if(cpuSelection == "r")
    {
        resultString = "win"
    }
    else if(cpuSelection == "p")
    {
        resultString = "draw"
    }
    else if(cpuSelection == "s")
    {
        resultString = "lose"
    }
}
else if(playerSelection == "s")
{
    if(cpuSelection == "r")
    {
        resultString = "lose"
    }
    else if(cpuSelection == "p")
    {
        resultString = "win"
    }
    else if(cpuSelection == "s")
    {
        resultString = "draw"
    }
}

as far as I know, this is the simplest form to educate students how to make it, do you have any ideas?

Thanks a lot

Answer 1

What about:

if (playerSelection == "r" && cpuSelection == "r") || (playerSelection == "p" && cpuSelection == "p") || (playerSelection == "s" && cpuSelection == "s") {
    resultString = "draw"
} else if (playerSelection == "r" && cpuSelection == "p") || (playerSelection == "p" && cpuSelection == "s") || (playerSelection == "s" && cpuSelection == "r") {
    resultString = "lose"
} else {
    resultString = "win"
}

// Edited, suggested by @Quince:

    if (playerSelection == cpuSelection) {
        resultString = "draw"
    } else if (playerSelection == "r" && cpuSelection == "p") || (playerSelection == "p" && cpuSelection == "s") || (playerSelection == "s" && cpuSelection == "r") {
        resultString = "lose"
    } else {
        resultString = "win"
    }

Answer 2

let winArray = ["rs", "pr", "sp"]

func getResult(playerSelection:String, cpuSelection:String) -> String {
    if playerSelection == cpuSelection {
        return "draw"
    }
    if winArray.contains(playerSelection + cpuSelection) {
        return "win"
    } else {
        return "lose"
    }
}
resultString = getResult(playerSelection, cpuSelection)

Using enums for safety:

enum GameResult {
    case Win
    case Draw
    case Lose
}

enum Selection {
    case Rock
    case Paper
    case Scissor
}

let winArray:[(Selection, Selection)] = [(.Rock, .Paper), (.Paper, .Scissor), (.Scissor, .Paper)]

func getResult(playerSelection:Selection, cpuSelection:Selection) -> GameResult {
    if playerSelection == cpuSelection {
        return .Draw
    }
    if winArray.contains({$0.0 == playerSelection && $0.1 == cpuSelection}) {
        return .Win
    } else {
        return .Lose
    }
}
let playerSelection:Selection = .Rock
let cpuSelection:Selection = .Scissor
let gameResult = getResult(playerSelection, cpuSelection: cpuSelection)

Answer 3

You should really use enumerations to manage your data values. You have 3 possibilities but you're working with explicit string literals. Enumerations also promote switch case statements which will be clearer in your case.

As you have a static set of options you also don't always need to test every one of them - if the first 2 aren't true then the 3rd must be. So you could assume it's a win and test for a loss or a draw.