3
\$\begingroup\$

Continuing with my VB.NET experience, I wrote a quick guess-the-number game. Any improvement suggestions?

Module GuessTheNumber
    Sub Main()
        Do
            PlayGame()
        Loop Until Not ExitGame()

        Console.ReadLine()
    End Sub

    Private Sub PlayGame()
        Dim minNumber As Integer = 1
        Dim maxNumber As Integer = 100
        Dim number As Integer = GetNum(minNumber, maxNumber)

        For guessCount = 1 To 6
            Dim guess As Integer = InputGuess(minNumber, maxNumber)

            Prompt(guess, number)

            If guess = number Then
                Exit Sub
            End If
        Next

        Console.WriteLine("Too bad, you ran out of guesses.")
    End Sub

    Private Function GetNum(ByVal Min As Integer, ByVal Max As Integer) As Integer
        Static Generator As System.Random = New System.Random()
        Return Generator.Next(Min, Max)
    End Function

    Private Function InputGuess(ByVal MinValue As Integer, ByVal MaxValue As Integer) As Integer
        Dim guess As Integer = -1

        Do
            Console.Write("Enter a guess between " + MinValue.ToString() + " and " + MaxValue.ToString() + ": ")
            If Not Integer.TryParse(Console.ReadLine(), guess) Then
                guess = -1
            End If
        Loop Until guess >= MinValue And guess <= MaxValue

        Return guess
    End Function

    Private Sub Prompt(ByVal guess As Integer, ByVal num As Integer)
        If guess > num Then
            Console.WriteLine("Your guess is too high.")
        End If
        If guess < num Then
            Console.WriteLine("Your guess is too low.")
        End If
        If guess = num Then
            Console.WriteLine("You won!")
        End If
    End Sub

    Private Function ExitGame() As Boolean

        Dim input As String = ""

        Do While input <> "yes" And input <> "no"
            Console.WriteLine("Would you like to play again ('yes' or 'no')?")
            input = Console.ReadLine().ToLower()
        Loop

        Return input = "yes"
    End Function
End Module
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Here are a few suggestions. n.b. I'm on my phone so can't run your code or see all of it on one screen.

1. Don't give boolean members negative names.

If you rename ExitGame() to something like KeepPlaying(), you'll have

Do
    PlayGame()
Loop While KeepPlaying()

instead of

Do
    PlayGame()
Loop Until Not ExitGame()

which (in my opinion) reads better. Once you get more conditions, having e.g. Not Invalid And Not NoValues starts to make it easier to introduce bugs because you need to think about the logic.

2. Consider using Console.ReadKey instead of Console.ReadLine.

Nobody wants to keep typing "yes" and pressing Enter - if you use ReadKey, you can just filter for "y" or "n".

3. Rename Prompt.

Its function is too give feedback, rather than to prompt the user. Maybe call it Respond, or GiveFeedback?

4. Use Else.

If guess > num Then
    Console.WriteLine("Your guess is too high.")
End If
If guess < num Then
    Console.WriteLine("Your guess is too low.")
End If
If guess = num Then
    Console.WriteLine("You won!")
End If

could be

If guess > num Then
    Console.WriteLine("Your guess is too high.")
Else If guess < num Then
    Console.WriteLine("Your guess is too low.")
Else If guess = num Then
    Console.WriteLine("You won!")
End If

The conditions are mutually exclusive, so once one is satisfied there's no need to check the others.

5. Consider letting the user specify the min and max values which bound the random number.

This would 'spice things up' and make the game more interesting (maybe). Use Math.Log(range, 2) as a guide to the number of guesses to allow the user.

6. Introduce a variable to represent the state of the user input

Currently you have:

Dim guess As Integer = -1

Do
    Console.Write("Enter a guess between " + MinValue.ToString() + " and " + MaxValue.ToString() + ": ")
    If Not Integer.TryParse(Console.ReadLine(), guess) Then
        guess = -1
    End If
Loop Until guess >= MinValue And guess <= MaxValue

Return guess

but you could instead go for something like:

Dim guess As Integer = -1
Dim validInput As Boolean = false

Do
    Console.Write("Enter a guess between " + MinValue.ToString() + " and " + MaxValue.ToString() + ": ")
    validInput = Integer.TryParse(Console.ReadLine(), guess)
Loop Until validInput And guess >= MinValue And guess <= MaxValue

Return guess

(Apologies if that doesn't build - I'm from C# land).

That way, you don't set the value of guess if it's invalid and the intent is clearer.

7 (maybe). Reuse the same instance of System.Random.

I think you're already doing this - my VB.NET knowledge is weak (again, C# land). The default constructor for Random seeds the generator with a time-dependant seed, so multiple instances created in (very) quick succession will give the same values.

\$\endgroup\$

Your Answer

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