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
asked Apr 4, 2015 at 0:05
\$\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.

answered Apr 4, 2015 at 10:08
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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