Cleaning up code

[Brad]

The following code works but I think could be improved.

Any suggestions would be appreciated!

Sub xxCodes()
Dim InpType As String
Dim tryagain As Long
startinput:
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "XX1" To "XX5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
Case Else
tryagain = MsgBox("Do you want to try again", vbYesNo)
If tryagain = 6 Then
GoTo startinput
Else
End
End If
End Select
End Sub

 

[Brad]

I think this is better - suggestions are welcome

Sub xxlCodes1()
Dim InpType As String
Dim tryagain As Long
tryagain = 6
Do Until tryagain = 7
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "xx1" To "xx5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
tryagain = 7
Case Else
tryagain = MsgBox("Incorrect school code do you want to try
again?", vbYesNo)
End Select
Loop
End Sub

 

[Jim Thomlinson]

Your second attempt is much better than your first. Always avoid goto's
except to invoke an error handling routine. Here is my take...

Sub xxlCodes1()
Dim InpType As String
Dim tryagain As Long

Do Until tryagain = vbNo
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "xx1" To "xx5"
With shtInput
.Range("State_Code") = InpType
.Range("product") = ""
End With
tryagain = vbNo
Case Else
tryagain = MsgBox("Incorrect school code do you want to try
again?", vbYesNo)
End Select
Loop
End Sub

It is not much different. I removed the 7 and replaced it with vbNo.
Additionally I added a with statement as that makes the code marginally more
efficient.

 

[Rick Rothstein \(MVP - VB\)]

This is probably how I would have written it...

Sub xxCodes()
Dim InpType As String
Dim shtInput As String
Do
InpType = InputBox("Enter the school code")
If InpType Like "[xX][xX][1-5]" Then
shtInput.Range("State_Code") = UCase(InpType)
shtInput.Range("product") = ""
Exit Do
End If
Loop While vbYes = MsgBox("Incorrect school code do you " & _
"want to try again?", vbYesNo)
End Sub

I do have one question though... your are referencing a worksheet variable
shtInput, but I don't see where you have set it at... are you doing it
through some kind of global setting or did you just forget to include it in
your code?

Rick

 

[Rick Rothstein \(MVP - VB\)]

Damn... I meant to leave the Dim shtInput statement out until you answered
the question (if it is set globally, the Dim statement isn't necessary)...

Sub xxCodes()
Dim InpType As String
Do
InpType = InputBox("Enter the school code")
If InpType Like "[xX][xX][1-5]" Then
shtInput.Range("State_Code") = UCase(InpType)
shtInput.Range("product") = ""
Exit Do
End If
Loop While vbYes = MsgBox("Incorrect school code do you " & _
"want to try again?", vbYesNo)
End Sub


Rick

This is probably how I would have written it...

Sub xxCodes()
Dim InpType As String
Dim shtInput As String
Do
InpType = InputBox("Enter the school code")
If InpType Like "[xX][xX][1-5]" Then
shtInput.Range("State_Code") = UCase(InpType)
shtInput.Range("product") = ""
Exit Do
End If
Loop While vbYes = MsgBox("Incorrect school code do you " & _
"want to try again?", vbYesNo)
End Sub

I do have one question though... your are referencing a worksheet variable
shtInput, but I don't see where you have set it at... are you doing it
through some kind of global setting or did you just forget to include it
in your code?

Rick

I think this is better - suggestions are welcome

Sub xxlCodes1()
Dim InpType As String
Dim tryagain As Long
tryagain = 6
Do Until tryagain = 7
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "xx1" To "xx5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
tryagain = 7
Case Else
tryagain = MsgBox("Incorrect school code do you want to try
again?", vbYesNo)
End Select
Loop
End Sub

 

[Brad]

Thanks!

I'm going to have to look up what

If InpType Like "[xX][xX][1-5]" Then
is really doing - I don't see why the [xX] has to be repeated twice. If you
could shed some light I would appreciate it.

my second dim statement was
Dim tryagain As Long
not
Dim shtInput As String - whch you have - don't know were you got this?
--
Wag more, bark less

This is probably how I would have written it...

Sub xxCodes()
Dim InpType As String
Dim shtInput As String
Do
InpType = InputBox("Enter the school code")
If InpType Like "[xX][xX][1-5]" Then
shtInput.Range("State_Code") = UCase(InpType)
shtInput.Range("product") = ""
Exit Do
End If
Loop While vbYes = MsgBox("Incorrect school code do you " & _
"want to try again?", vbYesNo)
End Sub

I do have one question though... your are referencing a worksheet variable
shtInput, but I don't see where you have set it at... are you doing it
through some kind of global setting or did you just forget to include it in
your code?

Rick

I think this is better - suggestions are welcome

Sub xxlCodes1()
Dim InpType As String
Dim tryagain As Long
tryagain = 6
Do Until tryagain = 7
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "xx1" To "xx5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
tryagain = 7
Case Else
tryagain = MsgBox("Incorrect school code do you want to try
again?", vbYesNo)
End Select
Loop
End Sub

 

[Brad]

Thank you (again)!

When I first started, I didn't know how to structure it. I try to avoid
"ends" and GOTO's as much as possible. But sometimes it is easier to change
something that works than coming up with the original idea....

 

[Brad]

Read about Like - "like" it.

Thank you very much.

This is probably how I would have written it...

Sub xxCodes()
Dim InpType As String
Dim shtInput As String
Do
InpType = InputBox("Enter the school code")
If InpType Like "[xX][xX][1-5]" Then
shtInput.Range("State_Code") = UCase(InpType)
shtInput.Range("product") = ""
Exit Do
End If
Loop While vbYes = MsgBox("Incorrect school code do you " & _
"want to try again?", vbYesNo)
End Sub

I do have one question though... your are referencing a worksheet variable
shtInput, but I don't see where you have set it at... are you doing it
through some kind of global setting or did you just forget to include it in
your code?

Rick

I think this is better - suggestions are welcome

Sub xxlCodes1()
Dim InpType As String
Dim tryagain As Long
tryagain = 6
Do Until tryagain = 7
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "xx1" To "xx5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
tryagain = 7
Case Else
tryagain = MsgBox("Incorrect school code do you want to try
again?", vbYesNo)
End Select
Loop
End Sub

 

[Rick Rothstein \(MVP - VB\)]

Think of the Like operator as a miniature version of a Regular Expression
parser... it works by comparing text strings to text "patterns". The
asterisk (0 or more characters) and question mark (a single character) are
pretty well-known wildcards and Like makes use of them. In addition, you can
specify multiple characters for any given character position. That is what
the square brackets do... signify a the characters that may appear at the
given position within the pattern string. In my code's case, the choice of
characters is between an lower case X and an upper case X (that is why it
appears twice, once in lower case and once in upper case). So in my code,
the InpType variable is being checked to see if the first two characters are
either a lower case X or an upper case X (that is why [xX] appears twice,
once for each character position) followed by one of the digits 1 through 5.
You should check out the Like operator in the help files... it is a power
built-in tools that comes in quite handy in many situations.

Rick

Thanks!

I'm going to have to look up what

If InpType Like "[xX][xX][1-5]" Then
is really doing - I don't see why the [xX] has to be repeated twice. If
you
could shed some light I would appreciate it.

my second dim statement was
Dim tryagain As Long
not
Dim shtInput As String - whch you have - don't know were you got this?
--
Wag more, bark less

This is probably how I would have written it...

Sub xxCodes()
Dim InpType As String
Dim shtInput As String
Do
InpType = InputBox("Enter the school code")
If InpType Like "[xX][xX][1-5]" Then
shtInput.Range("State_Code") = UCase(InpType)
shtInput.Range("product") = ""
Exit Do
End If
Loop While vbYes = MsgBox("Incorrect school code do you " & _
"want to try again?", vbYesNo)
End Sub

I do have one question though... your are referencing a worksheet
variable
shtInput, but I don't see where you have set it at... are you doing it
through some kind of global setting or did you just forget to include it
in
your code?

Rick

I think this is better - suggestions are welcome

Sub xxlCodes1()
Dim InpType As String
Dim tryagain As Long
tryagain = 6
Do Until tryagain = 7
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "xx1" To "xx5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
tryagain = 7
Case Else
tryagain = MsgBox("Incorrect school code do you want to try
again?", vbYesNo)
End Select
Loop
End Sub



:

The following code works but I think could be improved.

Any suggestions would be appreciated!

Sub xxCodes()
Dim InpType As String
Dim tryagain As Long
startinput:
InpType = UCase(InputBox("Enter the school code"))
Select Case InpType
Case "XX1" To "XX5"
shtInput.Range("State_Code") = InpType
shtInput.Range("product") = ""
Case Else
tryagain = MsgBox("Do you want to try again", vbYesNo)
If tryagain = 6 Then
GoTo startinput
Else
End
End If
End Select
End Sub