3
\$\begingroup\$

I wrote logic for "Wordle" game but I think it's too heavy and not very readable.
Is there any way to improve it?

Simple wordle game with conditions:
1) Green colour - "G" if string1 and string2 indices values are equal.
2) Yellow colour - "Y" if string2 indices value appears in string1.
3) else statement Grey colour - "."
4) Every letter from string1 used for colour only once.

My code:

let mstr1 = readLine()!
let mstr2 = readLine()!

var string1 = mstr1.compactMap { $0.asciiValue }
var string2 = mstr2.compactMap { $0.asciiValue }

var dict: Dictionary<UInt8, Int> = [:]

for element in Set(string1) {
    dict[element] = string1.filter { $0 == element }.count
}

var answer = Array(repeatElement(".", count: string1.count))

for i in 0 ..< string1.count {
    if string1[i] == string2[i] {
        answer[i] = "G"
        dict[string1[i]]! -= 1
    }
}

for i in 0 ..< string1.count {
    if dict[string2[i]] != nil && answer[i] != "G"{
        if dict[string2[i]]! > 0 {
            dict[string2[i]]! -= 1
            answer[i] = "Y"
        }
    }
    
    if dict.values.filter { $0 > 0 }.count <= 0 {
        break
    }
}

print(answer.joined())
Input: 
ABBEY
ALGAE
Output: 
G...Y
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

The logic itself looks good to me, but the readability of the program can be improved:

  • Put the logic to compute the response for some guess into a function and separate it from the I/O. That increases the clarity of the program and allows to add test cases more easily.

  • Use better variable names: What is string1 and string2? Which one is the correct answer and which one is the guessed word? What you call “answer” is (IMO) better called “response”.

  • The dictionary dict plays a crucial role in the algorithm, therefore some comment explaining what it exactly contains would be helpful.

Also the code can be simplified and improved, using Swift features like enumeration, dictionary subscripting with default values, optional binding, etc:

  • There is no need to convert the strings to ASCII. One reason to do that might be that you get an array which allows easy access by subscript. But often that is not needed, and one can enumerate the strings instead.

  • Dictionary<UInt8, Int> is more Swiftly written as [UInt8: Int]

  • Creating the dictionary with the character frequencies can be done simpler and more efficiently by enumerating the string once:

    var dict: Dictionary<UInt8, Int> = [:]
    for element in string1 {
        dict[element, default: 0] += 1
    }
    

    or even more concisely:

    var dict = string1.reduce(into: [:]) { (dict, char) in
        dict[char, default: 0] += 1
    }
    

    However: if the frequencies are counted inside the first loop (which tests for exact matches) then one can get rid of the forced unwrapping in dict[string1[i]]! -= 1.

  • This is fine

    var answer = Array(repeatElement(".", count: string1.count))
    

    but an alternative is

    var answer = string1.map { _ in "." }
    
  • Checking for existence of a value for a key in a dictionary is often better done with optional binding instead of if dict[...] != nil, in particular if the value is also needed, i.e.

    if dict[string2[i]] != nil && answer[i] != "G"{
        if dict[string2[i]]! > 0 {
            dict[string2[i]]! -= 1
            answer[i] = "Y"
        }
    }
    

    can be replaced by

    if let count = dict[string2[i]], count > 0, answer[i] != "G" {
        dict[string2[i]] = count - 1
        answer[i] = "Y"
    }
    

Putting it all together, the function could look like this:

func wordleResponse(guess: String, solution: String) -> String {

    var response = guess.map { _ in "." }

    // For each character `c` in the solution word, `frequencies[c]` is
    // the number of occurrences not yet matched by a character in the
    // guessed word.
    var frequencies = [Character: Int]()

    for (position, (guessedChar, solutionChar)) in zip(guess, solution).enumerated() {
        if guessedChar == solutionChar {
            response[position] = "G"
        } else {
            frequencies[solutionChar, default: 0] += 1
        }
    }
    
    for (position, guessedChar) in guess.enumerated() {
        if let count = frequencies[guessedChar], count > 0, response[position] != "G" {
            response[position] = "Y"
            frequencies[guessedChar] = count - 1
        }
    }
    
    return response.joined()
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.