CODEWITHSUNDEEP
iosiphoneswift
DeyaEldeen
DeyaEldeen

Reputation: 11857

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

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

Upvotes: 8

Views: 291

Answers (5)

J.Wang
J.Wang

Reputation: 1236

What about this:

let scores = ["s", "r", "p"]
let playerScore = scores.indexOf(playerSelection)!
let cpuScore = scores.indexOf(cpuSelection)!
resultString = playerScore > cpuScore ? "win" : playerScore == cpuScore ? "draw" : "lose"

Or in this particular context (very tricky):

resultString = playerSelection < cpuSelection ? "win" : playerSelection == cpuSelection ? "draw" : "lose"

Upvotes: 1

Crazy Developer
Crazy Developer

Reputation: 3464

Try nested trinary operator to check this in one line using the below code 

resultString = ( playerSelection == "r") ? (cpuSelection == "r" ? "draw" : (cpuSelection == "p") ? "lose" : "win") : ((( playerSelection == "p") ? (cpuSelection == "r" ? "win" : ((cpuSelection == "p") ? "draw" : "lose")):((cpuSelection == "r" ? "lose" : ((cpuSelection == "p") ? "win" : "draw")))))

you can use enum and array circulation logic.

Please check the below code. 

enum GameResult : Int {
    case None = 0
    case Win = 1
    case Draw = 2
    case Lose = 3
}

enum Selection : Int{

    case Paper = 1
    case Rock = 2
    case Scissor = 3
}

var pSelect = Selection.Paper;
var cSelect = Selection.Rock;
var result = GameResult.None

var playerRotation = (pSelect.rawValue) % 3 + 1;

if(pSelect.rawValue == cSelect.rawValue){
    result = GameResult.Draw
}
else if(playerRotation == cSelect.rawValue){
    result = GameResult.Win
}
else{
    result = GameResult.Lose
}

Upvotes: 5

marosoaie
marosoaie

Reputation: 2381

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)

Upvotes: 9

Greg
Greg

Reputation: 25459

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"
    }

Upvotes: 9

Wain
Wain

Reputation: 119041

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.

Upvotes: 6

Related Questions