InputBox and UBound input validation

[sully213]

I'm putting together a simple default printer selection script and of course I want to validate the user input to make sure their selection falls within the range of the installed printers array. However, I cannot for the life of me see what I'm doing wrong in the upper limit check. When I have that block of code enabled no matter what I enter it's getting flagged as the value is too high. If I comment out the upper level check (lines 8-10 below) then everything else works as expected. What am I missing?

Here is the validation code:

VBScript Code

Double-click the code block to select all.

Do
	strNewDefaultPrinter = InputBox(BuildMenu, "Select a New Default Printer")
	If IsEmpty(strNewDefaultPrinter) <> False Then
		WScript.Echo "You must make a selection."
		strGoodInput = False
	ElseIf IsNumeric(strNewDefaultPrinter) <> True Then
		WScript.Echo strNewDefaultPrinter & " is not a valid number. Try again."
	ElseIf strNewDefaultPrinter > UBound(arrInstalledNewServerPrinters)+1 Then
		WScript.Echo "Your selected value is too high. Try again."
		strGoodInput = False
	ElseIf strNewDefaultPrinter < 1 Then
		WScript.Echo "Your selected value is too low. Try again."
		strGoodInput = False
	Else
		strGoodInput = True
	End If
Loop Until strGoodInput = True

BuildMenu is a Sub I use to build a selection menu of currently installed printers numbered 1 through x, strGoodInput is initialized as True and is set to False if any of the validation checks fail. The user is prompted to enter the number corresponding to the printer listed in the menu.

If you need more of the code let me know, but this snippet should be good enough to get the idea across.

[jvierra]

This works fine for me:

VBScript Code

Double-click the code block to select all.

arrInstalledNewServerPrinters = Array(1,2,3,4,5,6)
Do
    strNewDefaultPrinter = InputBox(BuildMenu, "Select a New Default Printer")
    If IsEmpty(strNewDefaultPrinter) Then
        WScript.Echo "You must make a selection."
    ElseIf Not IsNumeric(strNewDefaultPrinter) Then
        WScript.Echo strNewDefaultPrinter & " is not a valid number. Try again."
    ElseIf (strNewDefaultPrinter - 1) > UBound(arrInstalledNewServerPrinters) Then
        WScript.Echo "Your selected value is too high. Try again."
    ElseIf strNewDefaultPrinter < 1 Then
        WScript.Echo "Your selected value is too low. Try again."
    Else
        bGoodInput= True
    End If
Loop Until bGoodInput

[jvierra]

This may be more obvious:

VBScript Code

Double-click the code block to select all.

arrInstalledNewServerPrinters = Array("one","two","three","four","five","six")
nPrinters = UBound(arrInstalledNewServerPrinters) + 1
On Error Resume Next
Do
    number = Cint(InputBox(BuildMenu, "Select a New Default Printer"))
    If Err.Number = 13 Then
        WScript.Echo "Please enter a valid number. Try again."
    ElseIf number > 0 And number <=  nPrinters THen
        bGoodInput = True
    Else
        WScript.Echo "Valid Range Is 1 To " & UBound(arrInstalledNewServerPrinters) + 1 & " - try again!"
    End If
    
Loop Until bGoodInput
On Error GoTo 0
WScript.Echo "You entered:" & arrInstalledNewServerPrinters(number - 1)

[sully213]

Thanks, though your code didn't work 100% seeing the CInt ahead of the UBound put me on the right path. I ended up having to use CStr in front of UBound when making the comparison. I was then able to discover that this only reliably tested up to 4-digit numbers (so 9999 would test as too high, but 10000 or above threw an out of range error), so I also added a check to the too high logic for lengths greater than 4.

Here is the fully working code:

VBScript Code

Double-click the code block to select all.

Do
	strNewDefaultPrinter = InputBox(BuildMenu, "Select a New Default Printer")
	If IsEmpty(strNewDefaultPrinter) <> False Then
		WScript.Echo "You must make a selection."
		bGoodInput = False
	ElseIf IsNumeric(strNewDefaultPrinter) <> True Then
		WScript.Echo strNewDefaultPrinter & " is not a valid number. Try again."
		bGoodInput = False
	ElseIf strNewDefaultPrinter > Cstr(UBound(arrInstalledNewServerPrinters) + 1) Or Len(strNewDefaultPrinter) > 4  Then
		WScript.Echo "Your selected value is too high. Try again."
		bGoodInput = False
	ElseIf strNewDefaultPrinter < 1 Then
		WScript.Echo "Your selected value is too low. Try again."
		bGoodInput = False
	Else
		bGoodInput = True
	End If
Loop Until bGoodInput = True

[jvierra]

Why would you care about too high or too low. A number is either good or bad. Size is not important. Are you trying to make the user guess at the printer by randomly entering numbers.

Just display the list and input a number. Check it against the list. It either returns a value or is not a good number.

The fastest way to check an index into an array is to just use it.

Here is another functional approach that uses the system and the program state as well as what we know about the mathematics of the requirement.

VBScript Code

Double-click the code block to select all.

arrInstalledNewServerPrinters = Array("one","two","three","four","five","six")

On Error Resume Next
Do
    input=CInt(InputBox("Menu here", "Select a New Default Printer"))
    If input= 0 Then WScript.Quit ' escape clause
    selection = input - 1
    If selection <= UBound(arrInstalledNewServerPrinters) Then
        result= arrInstalledNewServerPrinters(selection)
        If Err.Number <> 0 Then  ' TOO HIGH
            WScript.Echo "Not a good printer selection"
        Else
            Exit Do
        End If
    End If
Loop While True
WScript.Echo "You entered:" & result