Word-guessing game

Question

The objective of the game is to guess a five-lettered word in five guesses or less. When the user (player) enters a word, the application will inform the user whether their guess was correct or not. If the user guesses the correct word the application will simply "move to the next level" and prompt the user to guess another word. If the user incorrectly guesses the word, the application will remove an attempt from the user and will give a hint based on which letters they guessed correctly. The letters the user have guessed correctly can be displayed in two ways: (Green - correct letter, correct index) (Yellow - correct letter, incorrect index)

I want to find out whether my logic is efficient or not for the application. I'd appreciate any feedback. Thank you.

fun main(args: Array<String>) {
    Menu()
}

// Game Menu
fun Menu() {

    println("******************************\n" +
            "Welcome to WordGame!\n" +
            "******************************")
    println("Guess the correct word in\n" +
            "five attempts or less\n" +
            "******************************")
    println("Type your name to play")
    val player = readln()
    WordGame(player)
}

// Game Function 
fun WordGame(player : String) {

    var level : Int = 1
    var attempts : Int = 5

    // Array to hold random words
    val words: Array<String> = arrayOf("mayor", "train", "watch", "movie", "photo");
    var word : String  = words.random() // Gets random word in array words

    // Loops around the game 
    while (true) {

        println("******************************")
        println(player + " - Level " + level + " - Attempts " + attempts)
        println("******************************")
        println("Guess the word:")
        val answer : String = readln()

        // Checks for correct word
        if(CheckIndex(word, answer).size == 5) {

            level = level + 1 // adds level
            println("Correct!")
            attempts = 5 // resets the players attempts
            word = words.random()

            // Ends game when level 6 is reached
            if (level == 6) {

                println("Well done! You have beaten WordGame.\n" +
                        "Press enter to return to menu")
                readln()
                Menu()
            }
        }
        else {
            attempts = attempts - 1 // Removes an attempt

            // Ends game when attempts reach 0
            if (attempts == 0) {

                println("You've Lost at WordGame!\n" +
                        "Press enter to return to menu")
                readln()
                Menu()
            }

            var index = CheckIndex(word, answer).joinToString()
            var letters = CheckLetters(word, answer).joinToString()

            println("******************************\n" +
                    "Green: " + index + "\n" +
                    "Yellow: " + letters + "\n" +
                    "******************************")
            println("Remaing attempts: " + attempts)
            println("Press enter to try again")
            readln()
        }
    }
}

// Function to check index
fun CheckIndex(word: String, answer: String) : ArrayList<Char> {

    val index = ArrayList<Char>()

    var i = 0
    // Loops around the length of the guessed word
    while (i < answer.length) {

        // Checks against the char of guessed word
        // and correct word
        if(answer.get(i) == word.get(i)) {
            index.add(answer.get(i)) // if a char is equal, adds to an arraylist
        }
        i++
    }
    return index
}

// Function to check correct letters
fun CheckLetters(word: String, answer: String) : ArrayList<Char> {

    val letters = ArrayList<Char>()

    var i = 0
    while (i < answer.length) {

        // Ensures the 'Green' letters are
        // not seen as 'Yellow'
        if(answer.get(i) != word.get(i)) {

            // Checks against each letter in the correct word
            // I feel that this is a bit clunky as I would prefer
            // to not hard-code indexe
            when (answer.get(i)) {
                word.get(0) -> letters.add(answer.get(i))
                word.get(1) -> letters.add(answer.get(i))
                word.get(2) -> letters.add(answer.get(i))
                word.get(3) -> letters.add(answer.get(i))
                word.get(4) -> letters.add(answer.get(i))
            }
        }
        i++
    }
    return letters
}

Answer 1

First off, this isn't a bad attempt, however there are some bugs:

What happens when the user enters a word with less than 5 letters? What if the player enters uppercase letters?

Also the returned hints can be unhelpful/wrong in some cases. For example, with the secret word movie: if the player enters where the game will display e as green without any information which e is correct. And it will additionally display e as yellow, which isn't correct, since movie only contains one e which is already being shown as green.

---

You are inconsistently following Kotlin code conventions.

• Function names should begin with a lower case letter.
• There shouldn't be a space before the colon when declaring types.
• There should be a space between keywords (if, while, etc.) and the following parenthesis.
• etc.

Your IDE (specifically IntelliJ) can format the code for you.

---

You are purely using a procedural programming style and not really using the features of Kotlin, e.g. object-oriented and functional programming. That's not specifically wrong, but you are missing out on its strengths.

You could, for example, use multiline (raw) string literals and string templates:

println("""
    |******************************
    |${player} - Level ${level} - Attempts ${attempts}
    |******************************
    |Guess the word
    """.trimMargin())

Another suggestion would be to use Kotlin classes, for example its collections, instead of the Java-specific ones (such as ArrayList).

This maybe too advanced, but a Kotlin-specific way to implement checkIndex could be:

fun checkIndex(word: String, answer: String): List<Char> =
    word.zip(answer).filter { (a, b) -> a == b }.map { it.first }

---

It's not really optimal to call Menu() from inside WordGame(), especially with the infinite loop. After several rounds you'll have a Menu() call inside a WordGame() call inside a Menu() call inside a WordGame() call inside a Menu() call inside a WordGame() call inside a Menu() call inside a WordGame() call, etc.

Better would be to break out of the loop in WordGame() and instead have an additional loop inside Menu() (preferably one that the user can exit):

fun Menu() {
    while (true) {
        // ...
        println("Type your name to play")
        val player = readln()
        if (player.isBlank()) break
        WordGame(player)
    }
}

In WordGame() then use break (or return to exit the function directly) instead of calling Menu().

---

Instead of the when just add another loop over the letters of word and compare each to answer.get(i).

Answer 2

As @RoToRa pointed out an issue,

For example, with the secret word movie if the player enters where the game will display e as green without any information which e is correct. And it will additionally display e as yellow, which isn't correct, since movie only contains one e which is already being shown as green.

I would like to suggest a solution to improve your logic, how about you print the exact word again, which in the above example is where but with the correct letters colored according to your color code. In order to do this, you will have to check each letter of the answer (so it has to be in a loop) and print accordingly using the ANSI escape sequence like this for instance:

//storing escape codes in variables for ease
val yellow: String = "\e[33m"
val green: String = "\e[32m"

//"\e[m" resets text color 
val colorOff: String = "\e[m"

//if(letter is correct but wrong index)
print(yellow+letter+colorOff)
//else if(letter is correct && correct index)
print(green+letter+colorOff)
//else
print(letter)   //printing the wrong letter without any color

However, it's worth noting that the escape code "\e" may not be recognized by all terminals or operating systems. So the behavior of the code that uses this variable may not be consistent across all environments.

Also, make sure you print the color code beforehand in the execution of the program explaining it to the user. By the way, I think the code you provided lacks the hints to guess the word. Glad if I could help.

Answer 3

I'm not checking for bugs in your logic. I suspect you are not handling green and yellow letters correctly when there's more than one of the same letter in the target word, but I just wanted to focus on the checkLetters() function since you commented that you think it might be clunky, and you're right about that. So this answer is about how this function could be more elegantly written without changing its behavior.

The below will get you the exact same behavior for the function, any existing bugs still included!

---

• It's Kotlin convention to start function names with a lower-case letter. This is kind of important for code readability, since Kotlin doesn't have a new keyword that helps distinguish between constructor calls and regular function calls. If you follow convention, constructor calls stand out because they start with a capital letter.
• You can use the buildList function to more elegantly write this function. It allows you to directly call MutableList functions within your lambda, and then at the end simply returns a List. You don't need to specify ArrayList since you are not further mutating this list after this function returns.
  • Also, in idiomatic Kotlin or Java, you would rarely ever use ArrayList as a return type instead of just List/MutableList. The specific type ArrayList generally would be useless extra information to whoever is calling this function. This is related to the principle of encapsulation.)
• string.get(i) is less idiomatic than string[i].
• Your while loop with externally declared i that is internally incremented is convoluted. You could get the same functionality using for (i in answer.indices).
• You are using answer.get(i) many times. When this happens, you should pull it out into a variable first so you aren't calling that getter repeatedly.

So, implementing the above points before moving on, we have:

fun checkLetters(word: String, answer: String): List<Char> = buildList {
    for (i in answer.indices) {

        // Ensures the 'Green' letters are
        // not seen as 'Yellow'
        val thisChar = answer[i]
        if(thisChar != word[i]) {
            when (thisChar) {
                word.get(0) -> add(thisChar)
                word.get(1) -> add(thisChar)
                word.get(2) -> add(thisChar)
                word.get(3) -> add(thisChar)
                word.get(4) -> add(thisChar)
            }
        }
    }
}

Now, look closely at your when statement. You are doing the exact same thing in all branches! We can phrase the logic of this section of code as "add thisChar to the list if it matches any of the chars in word. This can be more simply achieved using word.contains, which can also be expressed as in word. So we can update the function as follows to get the exact same result:

fun checkLetters(word: String, answer: String): List<Char> = buildList {
    for (i in answer.indices) {

        // Ensures the 'Green' letters are
        // not seen as 'Yellow'
        val thisChar = answer[i]
        if(thisChar != word[i] && thisChar in word) {
            add(thisChar)
        }
    }
}

As an alternative way of doing this, instead of using buildList and manually iterating and adding to the list, you could use Kotlin's Iterable operators (sometimes referred to as functional programming). This is equivalent behavior to the above code:

fun checkLetters(word: String, answer: String): List<Char> = 
    answer.filterIndexed { i, c -> c != word[i] && c in word }.toList()

Likewise, your checkIndex() function could also be a one-liner:

fun checkIndex(word: String, answer: String): List<Char> =
    answer.filterIndexed { i, c -> c == word[i] }.toList()