0
\$\begingroup\$

An exercise asks that we extend the below random number generator app to print to the console how many times the user guessed before guessing the correct random number.

I first declared what I call a counter variable numberOfGuesses inside the keepPlaying while loop and then inside the continueGuessing while loop I increment this number by 1 before I check the user's guess against the random number generated. This works, but then I tried declaring numberOfGuesses at the top of the program and commented out the former placement, as you can see below, and this also worked.

My question is, which is the better way of these two to solve this, and is there another even better way to solve this problem by using another loop perhaps or would that be silly/overkill/not workable?

var randomNumber = 1
var numberOfGuesses = 0
var continueGuessing = true
var keepPlaying = true
var input = ""

while keepPlaying {
    //get a random number between 0 - 100
    randomNumber = Int(arc4random_uniform(101)) 
    print("The random number to guess is: \(randomNumber)")

    //var numberOfGuesses = 0 //can go here, or in global variables up top!

    while continueGuessing { //
        print("Pick a number between 0 and 100.")
        //get keyboard input, and trim the new line
        input = String(bytes: FileHandle.standardInput.availableData, encoding: .utf8)!
        input = input.trimmingCharacters(in: .whitespacesAndNewlines)

        if let userGuess = Int(input) {
            numberOfGuesses += 1
            if userGuess == randomNumber {
                continueGuessing = false //terminator for this while loop
                print("Correct number!")
                print("You have made this number of attempts to get it right: \(numberOfGuesses)")
            } else if userGuess > randomNumber {
                //user guessed too high
                print("Your guess is too high!")
            } else {
                //no reason to check if userGuess < randomNumber. It has to be.
                print("Your guess is too low!")
            }
        } else {
            print("Invalid guess, please try again.")
        }
}

print("Play again? Y or N")
    input = String(bytes: FileHandle.standardInput.availableData,encoding: .utf8)!
    input = input.trimmingCharacters(in: .whitespacesAndNewlines)

    if input == "N" || input == "n" {
        keepPlaying = false //a way to exit while loop
        print("You have played the game this number of times: \(numberOfGuesses)")

    }
    continueGuessing = true
}
\$\endgroup\$
2
\$\begingroup\$

As a general rule, you want to use the narrowest possible scope for all variables. We do this so that:

  1. The declaration is close to where we use it, making it easier to reason about why the variable exists.

  2. It minimizes the chance of unintended consequences where the variable is accidentally updated somewhere else.

In this case, it’s so trivial that it doesn’t matter that much, but as your code increases in complexity, this becomes increasingly important. This practice of restricting variables to the narrowest possible scope becomes a central precept in writing safe code: It’s much harder to have unintended consequences or accidentally state changes if variables have narrow scopes.

So, if numberOfGuesses represents how many guesses it took you to guess a particular random number, then it should be inside the keepPlaying loop. But if you also wanted to keep track of the total number of guesses for all the times you played, then that (probably best kept track in another variable, perhaps, totalNumberOfGuesses) would be outside this outer loop.

By the way, while you’re at it, the continueGuessing and randomNumber, belong inside this keepPlaying loop, too.

By the way,

  1. If you define variables only used within the scope of a loop, it saves you from having to reset that variable for the next iteration. For example, right now you are resetting continueGuessing back to true as the last step in the keepPlaying loop; if you define continueGuessing within the loop, then you wouldn’t have to reset it again at the end of the loop, resulting in simpler code.

In your code, you make reference to “globals”. As a general rule, we try to minimize the use of globals because as as apps scale, globals can make it hard to test, harder to reason about state changes, etc. See Why is Global State so Evil or google “globals are evil”.


Some unrelated observations:

  1. Rather than

    let randomNumber = Int(arc4random_uniform(101))
    

    I’d instead recommend:

    let randomNumber = Int.random(in: 0...100)
    

    It is equivalent, but arguably is more intuitive. (This would be especially true if you wanted to go from 1 to 100 instead, the arc4random_uniform code would become even less intuitive, whereas random(in: 1...100) is perfectly natural and trivial to reason about.

  2. When you have a loop that you are always going to do at least once, rather than a while loop, we’d often consider repeat-while. E.g. rather than

    var shouldPlayAgain = true
    while shouldPlayAgain {
        ...
    
        shouldPlayAgain = ...
    }
    

    You might do:

    var shouldPlayAgain: Bool
    repeat {
        ...
    
        shouldPlayAgain = ...
    } while shouldPlayAgain
    

    Technically the latter is a tad more efficient, but the main benefit is that it makes your intent (to check at the end) absolutely clear.

  3. At the end, you appear to want to be printing number of times the game was played, but you’re printing the number of guesses.

    E.g. if you played twice, the first time requiring 3 guesses and the second time requiring 4 guesses, do you want to say “you played 2 times” or do you want to say “you guessed a total of 7 times”?

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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