N
N
Natalia Ugolnikova2021-07-28 15:55:49
code review
Natalia Ugolnikova, 2021-07-28 15:55:49

May I ask you for a code critique?

Good day to all! I recently started learning Kotlin. I heard many times that you need to publish your code in order to improve your skills, so I decided to post my code.

I wrote a game "Rock, Paper, Scissors" where the user plays with the computer. At the beginning, the user is asked if he wants to play the game. If the user refuses, the program terminates accordingly. If the user agrees, a multiple-choice entry appears where the user must enter numbers from 1 to 3 to choose between rock, paper, and scissors. The next step is to compare the user's and the computer's responses to determine who wins or if it's a tie. After the comparison, the result is displayed on the command line. After the result, the user is asked again if he wants to play again. Thus, everything is repeated until the user answers in the negative.

Below I have attached the code of the game so that you can see and tell me where I have errors, is the code readable, is there enough information in the names of the variables (because I heard that the names should be as informative as possible).

Thank you in advance for your attention!

val options = mapOf(1 to "Rock", 2 to "Paper", 3 to "Scissors")

/*варианты ответов на вопросы о проведении игры
с ними будет происходить сравнение ответа пользователя*/

    val positiveAnswer = "y"
    val negativeAnswer = "n"

//запрос на проведение игры
    println("Hi! Here is the \"Rock.Paper.Scissors.\" game. Do you want to play? [Y] or [N]")

//ответ пользователя
    var playGameAnswer = readLine()

//зацикливаю программу до тех пор, пока пользователь не введет корректный ответ

    while ((negativeAnswer != playGameAnswer) || (positiveAnswer != playGameAnswer))
    {
        when (playGameAnswer)
        {

//если польователь отвечает положительно, то начинается игра
            positiveAnswer ->
            {
                val gameChose = getGameChose(options)
                val userChoice = getUserChose(options)
                getResult(gameChose, userChoice)
                println("Do you want to play any more?")
                playGameAnswer = readLine()
            }

//при отрицательном ответе программа завершится
            negativeAnswer ->
            {
                println("Oh..so sad. So, goodbye")
                break
            }

//в случае некорректного ввода будет задан повторный вопрос с указанием вариантов ответа
            else ->
            {
                println("Please, enter [Y] or [N]")
                playGameAnswer = readLine()
            }

        }
    }
}

/**
 * Функция сравнения и вывода результата игры
 */
fun getResult(gameChose: String, userChoice: String) {
    when
    {
        userChoice == gameChose -> println("Tie")

        (userChoice == "1" && gameChose == "3") ||
        (userChoice == "2" && gameChose == "1") ||
        (userChoice == "3" && gameChose == "2") ->
            println("You win!")

        else -> println("You lose.")
    }


/**
 * Функция для ввода варианта ответа пользователем
 */
fun getUserChose(options: Map<Int, String>): String {

//флаг с помощью которого будем делать зацикливание
    var isValidChoice = false

    var userChoice = ""

//вывод на экран вариантов ответа

    println("Chose one of this (write a number): ")
    for ((key, value) in options)
    {
        println("$key. $value")
    }

    while (!isValidChoice)
    {
        val userInput = readLine()

        //обрабатываю исключение в случае ввода букв, а не цифр
        try
        {
            if (userInput != null && options.contains(userInput.toInt()))
            {
                isValidChoice = true
                userChoice = userInput
            }

            if (userInput != null)
            {
                if (!options.contains(userInput.toInt()))
                    println("Please enter 1 or 2 or 3")
            }
        }
//в случае ввода букв, выведется сообщение 
            catch (e: Exception)
            {
                    println("Please enter 1 or 2 or 3")
            }

    }
    return userChoice
}

/**
 * Функция рандомного выбора компьютера
 */
    fun getGameChose(options: Map<Int, String>): String {
        val value = options.keys
        val random = (1 + Math.random() * value.size).toInt()
        when (random) {
            1 -> options[1]
            2 -> options[2]
            3 -> options[3]
        }
        return random.toString()
    }

Answer the question

In order to leave comments, you need to log in

2 answer(s)
F
foonfyrick, 2021-07-28
@foonfyrick

Everything that you have in ifs, I would put it in a separate function, there is no desire to understand what is happening there.

val gameChose = getGameChose(options)
val userChoice = getUserChose(options)
getResult(gameChose, userChoice)
println("Do you want to play any more?")
playGameAnswer = readLine()

println("Oh..so sad. So, goodbye")
break

this is all also in separate functions, it is more convenient for me to read when the function describes what is happening, and not to stumble over it every time and think about what is happening here.
I would advise you not to ask for advice, but to write the code as if you are writing it for your friends who are not very good at programming, but so that they understand what is happening here without going into details.

S
Sergey Vodakov, 2021-07-28
@WaterSmith

And how does it work?

(userChoice == "1" && gameChose == "3") ||
        (userChoice == "2" && gameChose == "1") ||
        (userChoice == "3" && gameChose == "2") ->

It's not beautiful! I would organize the choices as elements of a looped doubly linked list, then it would be enough to check the condition: "our choice is "to the left" or "to the right" of the player's choice. This will come in handy when scaling the game.
positiveAnswer ->
            {
                val gameChose = getGameChose(options)
                val userChoice = getUserChose(options)
                getResult(gameChose, userChoice)
                println("Do you want to play any more?")
                playGameAnswer = readLine()
            }

I would put this part of the code in a separate method, as well as the request to play the game.
PS If you have a question "What can be the scaling of such a simple game?", take a look at this picture:4ce06329743e68ed16ae90bfcdbfbf6a_ce_493x500x0x0.jpg

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question