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
1 Answer 1
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.