Make my method shorter

Question

I am looking for advice on making the following code shorter. Any help is appreciated.

public String determineWinner(String winner) {
    winner = "";
    if (playChoice.equals(compChoice))
        winner = "The result is a tie.";
    else if (compChoice.equals("R"))
    if (playChoice.equals("S") || playChoice.equals("s"))
        winner = "Computer wins because Rock beats Scissors.";
    else
        winner = "Player wins because Paper beats Rock";
    else if (compChoice.equals("S"))
    if (playChoice.equals("P") || playChoice.equals("p"))
        winner = "Computer wins because Scissors beats Paper.";
    else
        winner = "Player wins because Rock beats Scissors";
    else 
    if (playChoice.equals("R") || playChoice.equals("r"))
        winner = "Computer wins because Paper beats Rock.";
    else
        winner = "Player wins because Scissors beats Paper.";
    return winner;
}

Thanks

Answer 1

You code looks unclear from point of view of parameter usage and ignoring case difference is case of "The result is a tie.", so ...

Try this :).

public class testik {
    // No throws clause here
    public static void main(String[] args) {
        // Paper < Rock < Scissors < Paper
        System.out.println("Comp:S vs User:R = " + determineWinnerOtherWay("S","R")); //user win
        System.out.println("Comp:S vs User:P = " + determineWinnerOtherWay("S","P")); // com win

        System.out.println("Comp:P vs User:R = " + determineWinnerOtherWay("P","R")); //com win
        System.out.println("Comp:P vs User:S = " + determineWinnerOtherWay("P","S")); // user win

        System.out.println("Comp:R vs User:S = " + determineWinnerOtherWay("R","S")); //com win
        System.out.println("Comp:R vs User:P = " + determineWinnerOtherWay("R","P")); // user win
    }

    public static String code2Title(String code){
        if ("R".equals(code)) {
            return "Rock";
        } else if ("P".equals(code)) {
            return "Paper";
        } else {
            return "Scissors";
        }
    }

    public static String determineWinnerOtherWay(String compChoice, String playChoice) {
        String winner = "";
        String playerChoiceN = playChoice.toUpperCase();
        if (playChoice.equals(compChoice))
            winner = "The result is a tie.";
        int diff = compChoice.charAt(0) - playerChoiceN.charAt(0);
        if ( diff == 1 || diff == 2 || diff ==-3) {
            winner = "User win bacause:"+code2Title(playerChoiceN)+" wins " + code2Title(compChoice);
        } else {
            winner = "Computer wins because:"+code2Title(compChoice)+" wins " + code2Title(playerChoiceN);
        }
        return winner;
    }

}

Answer 2

Is this your homework assignment? Here's my solution in Python.

def winner(choice_P, choice_C):
    """ Given choices of Player and Computer (one of ['P', 'R', 'S']),
        return description of result of Rock, Paper, Scissors game."""
    names = ['Player', 'Computer']
    choices = ('Paper', 'Rock', 'Scissors')
    choice_pair = (choice_P + choice_C).upper()
    if choice_pair[0] == choice_pair[1]:
        return "The result is a tie."
    order = 'PRSP'
    index = order.find(choice_pair)
    if index == -1:                                 # not found
        names.reverse()
        index = order.find(choice_pair[::-1])       # (reversed)
        if index == -1:
            return "Invalid input, choices must be one of ['P', 'R', 'S']"
    return '%s wins because %s beats %s' % (names[0], 
                                            choices[index], choices[(index + 1) % 3])

Answer 3

I've written this in C#, but it should translate to Java easily.

I made determineWinner() call itself, reversing the order of player1 & player2. This eliminates fully half of the 'if else` maze.

// these are class level variables.
string winnerNotice = @"{0} wins. {1} beats {2}";
string drawNotice = @"Draw. Both players picked {0}";

// new structures to help clean up the determineWinner() method
public class Player { 
    public string Name;
    public choices choice;
}

public enum choices
{
    Paper = 1 , Scissors = 2 , Rock = 4
}

        public String determineWinner( Player player1, Player player2 ) {

            //tie
            if ( player1.choice == player2.choice ) 
                return string.Format( drawNotice ,player1.choice );

            // paper covers rock
            if ( player1.choice == choices.Paper && player2.choice == choices.Rock) 
                return string.Format(winnerNotice,player1.Name,player1.choice,player2.choice);

            // rock beats scissors beats paper
            if ( player1.choice > player2.choice ) 
                return string.Format( winnerNotice ,player1.Name ,player1.choice ,player2.choice );

            // player 1 did not win, did player 2 win?
            determineWinner(player2, player1);
        }
    }




}

Answer 4

Consider:

paper = 2
scissors = 3
rock = 5

Then your function could look like:

public int determineWinner(int player1, int player2)
{
    if(player1 != player2)
        if(player1 + player2 == 7)
            return Math.min(player1, player2);
        else
            return Math.max(player1, player2);
    else
        return 0;
}

Then the implementation is up to you.