0

The following code reveals a gap in my understanding. Can someone please tell me what it is and how to fix the code

Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
    Dim NumberToGuess, Answer As Integer
    NumberToGuess = InputBox("Enter a Secret Number Between 1 and 20!")
    While Answer <> NumberToGuess
        Answer = InputBox("Please enter your guess")
        If IsNumeric(Answer) = False Then MsgBox("That ain't no number")
        If Answer > NumberToGuess Then MsgBox("Too high thicko. Try Again")
        If Answer < NumberToGuess Then MsgBox("Too Low chump. Try Again")
    End While
    MsgBox("Well done you guessed the right number")
End Sub
Pradeep Kumar
  • 6,836
  • 4
  • 21
  • 47
RickTicky
  • 114
  • 8
  • guessing here since my VB.Net is ancient. You are using Answer <> NumberToGuess before "Answer" has a value. – Marvin Smit Aug 30 '14 at 09:48
  • 1
    If you add the error message perhaps is more clear what you are asking – Steve Aug 30 '14 at 09:49
  • This question reveals a gap in my understanding of the point of this question. Just throwing in some code without naming a specific problem, desired behaviour etc. Probably https://codereview.stackexchange.com/ would be a better place for these sort of questions. – Andreas Mar 17 '23 at 08:59

4 Answers4

2

There are many problems in this question and your question is vague about which one you are talking about. So I'll list all of them here.

  1. You declare both NumberToGuess and Answer as Integer and assign it the result of an InputBox. But InputBox can return anything (number or alpha). And it will error out as soon as you try to assign the user's input to NumberToGuess. And that's before you check whether it is number or not.
  2. If you have OPTION STRICT ON it will show you compilation error "Option Strict On disallows implicit conversions from 'String' to 'Integer'". Keeping OPTION STRICT ON is a good practice in general and helps avoid innocent looking mistakes. e.g. Here you are assigning String type to Integervariable, which it doesn't allow.
  3. You have used a While loop with InputBox. There is no way for user to cancel out of the game unless they give the correct answer. The Cancel button of InputBox won't work.
  4. All your If conditions will be evaluated irrespective of previous one. I assume you want only one of the Message Boxes to be shown at a time. So you may want to make use of ElseIf too.

To fix the problems, here we go:

Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
    Dim NumberToGuess As String    '<-- declare as string because we will hold the result of InputBox in it.
    Dim Answer As String = ""      '<-- declare as string because we will hold the result of InputBox in it.

    NumberToGuess = InputBox("Enter a Secret Number Between 1 and 20!")
    While Answer <> NumberToGuess
        Answer = InputBox("Please enter your guess")
        If String.IsNullOrEmpty(Answer) Then Exit While '<-- if user pressed cancel button in InputBox.
        If Not IsNumeric(Answer) Then
            MsgBox("That ain't no number")
        ElseIf CInt(Answer) > CInt(NumberToGuess) Then
            MsgBox("Too high thicko. Try Again")
        ElseIf CInt(Answer) < CInt(NumberToGuess) Then
            MsgBox("Too Low chump. Try Again")
        Else
            ' neither less nor more. so this is the correct answer.
            MsgBox("Well done you guessed the right number")
            Exit While
        End If
    End While
End Sub

The above code however is a big annoyance because a MessageBox, then InputBox, thenMessageBox, thenInputBox... To fix this, you can show the message in the InputBox itself.

Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
    Dim NumberToGuess As String    '<-- declare as string because we will hold the result of InputBox in it.
    Dim Answer As String = ""      '<-- replace with whatever answer you are expecting.
    Dim Message As String = "Please enter your guess"

    NumberToGuess = InputBox("Enter a Secret Number Between 1 and 20!")
    While Answer <> NumberToGuess
        Answer = InputBox(Message)
        If String.IsNullOrEmpty(Answer) Then Exit While '<-- if user pressed cancel button in InputBox.
        If Not IsNumeric(Answer) Then
            Message = "That ain't no number"
        ElseIf CInt(Answer) > CInt(NumberToGuess) Then
            Message = "Too high thicko. Try Again"
        ElseIf CInt(Answer) < CInt(NumberToGuess) Then
            Message = "Too Low chump. Try Again"
        Else
            ' neither less nor more. so this is the correct answer.
            MsgBox("Well done you guessed the right number")
            Exit While
        End If
    End While
End Sub
Pradeep Kumar
  • 6,836
  • 4
  • 21
  • 47
  • Thank you for this. Your solution is elegant and addresses all the flaws in my understanding. I much prefer the combination of error message and input box rather than one after the other – RickTicky Aug 30 '14 at 10:42
1

it's no big deal - you just missed some else ;)

private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
    Dim NumberToGuess, Answer As Integer
    NumberToGuess = InputBox("Enter a Secret Number Between 1 and 20!")
    While Answer <> NumberToGuess
        Answer = InputBox("Please enter your guess")
        If IsNumeric(Answer) = False Then MsgBox("That ain't no number")
        Else
            If Answer > NumberToGuess Then MsgBox("Too high thicko. Try Again")
            If Answer < NumberToGuess Then MsgBox("Too Low chump. Try Again")
        End If
    End While
    MsgBox("Well done you guessed the right number")

End Sub

The problem is that if you just write it like this the first condition is checked - if you don't input a number the message is shown and your next condition is checked - you obvious only want those to be checked if it is a number. So wrap it inside an Else Block

BTW: I don't think the missing value for Answer is an issue but I cannot check right now (My MonoDevelop on my Linux Distro has some issues with VB.net :( - debug your code to find out if it is please)

Random Dev
  • 51,810
  • 9
  • 92
  • 119
  • Thank you very much for such a prompt reply. I have been trying to play with an Else and ElseIF statement but without success. Your clear explanation helps me understand why the former is needed. However, when I make your adjustments, VB shows me a syntax error. The Else statement and the End IF statement are underlined in a blue squiggly line. Do you know why? Thanks again. – RickTicky Aug 30 '14 at 10:12
1

The problem is the return value of the InputBox function.
It returns a string and you try to assign it to an integer variable.
This forces the VB compiler to execute an implicit conversion from a string to an integer and, until you type valid numbers it works, but if you leave your inputbox empty, the implicit conversion fails and raises an InvalidCastException.

If you compile your code with Option Strict On this will be caught during the build.
Without it your program crashes at the assignment.
The same problem is present in the assignement of NumberToGuess

Dim NumberToGuess, Answer As Integer
While True
    Dim input = InputBox("Enter a Secret Number Between 1 and 20!")
    if IsNumeric(input) Then
        NumberToGuess = Convert.ToInt32(input)
        Exit While
    Else
        MsgBox("Not a valid number")
    End If
End While    
While Answer <> NumberToGuess
    Dim result = InputBox("Please enter your guess")
    If IsNumeric(result) = False Then 
       MsgBox("That ain't no number")
    Else
       Answer = Convert.ToInt32(result)
       If Answer > NumberToGuess Then MsgBox("Too high thicko. Try Again")
       If Answer < NumberToGuess Then MsgBox("Too Low chump. Try Again")
    End If
End While
MsgBox("Well done you guessed the right number")

I should also add that the answer from Mr Carsten Konig is rightly pointing to an error in your logical code flow.

Community
  • 1
  • 1
Steve
  • 213,761
  • 22
  • 232
  • 286
  • it unconditionally prints `Well done you guessed the right number` all times since it is out side the while –  Aug 30 '14 at 10:19
  • It never exits the loop if you don't type the correct number. – Steve Aug 30 '14 at 10:20
  • Ah, I see. Thank you very much Steve. Its amazing how a simple program can teach you so much. I really appreciate your help. Ditto everyone else who posted such useful comments – RickTicky Aug 30 '14 at 10:27
0

You can use:

 Dim NumberToGuess, Answer As Integer
        NumberToGuess = InputBox("Enter a Secret Number Between 1 and 20!")
        While Answer <> NumberToGuess
            Answer = InputBox("Please enter your guess")
            If IsNumeric(Answer) = False Then
                MsgBox("That ain't no number")
            Else
                Select Case Answer < NumberToGuess
                    Case True
                        MsgBox("Too Low chump. Try Again")
                    Case False
                        MsgBox("Too high thicko. Try Again")
                    Case Else
                        MsgBox("Well done you guessed the right number")
                End Select
            End If
        End While