Most Efficient Way To Write This Loop VB.Net

Question

Good afternoon all, I am beginning my first forays into programming and have decided to begin with VB.net as I can get VS2010 professional free through MS Dreamspark program.

I have been following some basic tutorials online and am now writing a small program that runs a loop to add all the numbers together between two numbers input by the user.

Below is the code I have written:

Public Class Form1
  Private Sub cmdAddNumbers_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles cmdAddNumbers.Click
    Dim NumberOne As Integer
    Dim NumberTwo As Integer
    Dim Result As Integer
    Dim i As Integer

    If Not IsNumeric(txtNumberOne.Text) Then
        MsgBox("Please Enter A Valid Number For Number One")
        txtNumberOne.Clear()
        Exit Sub
    ElseIf txtNumberOne.Text = 0 Then
        MsgBox("Please Enter A Valid Number For Number One")
        txtNumberOne.Clear()
        Exit Sub
    ElseIf txtNumberOne.Text > 0 And IsNumeric(txtNumberOne.Text) Then
        NumberOne = txtNumberOne.Text
    End If

    If Not IsNumeric(txtNumberTwo.Text) Then
        MsgBox("Please Enter A Valid Number For Number Two")
        txtNumberTwo.Clear()
        Exit Sub
    ElseIf txtNumberTwo.Text < NumberOne Then
        MsgBox("Please Enter A Valid Number For Number Two")
        txtNumberTwo.Clear()
        Exit Sub
    ElseIf txtNumberTwo.Text > NumberOne And IsNumeric(txtNumberTwo.Text) Then
        NumberTwo = txtNumberTwo.Text
    End If

    For i = NumberOne To NumberTwo
        Result = Result + i
    Next i

    txtResult.Text = Result
    txtNumberOne.Clear()
    txtNumberTwo.Clear()
  End Sub
End Class

Now, I am wondering if I have written the most efficent If statements to execute this code or if they can be written any simpler with AND/OR statements to possibly remove some of the ElseIf's.

Any insight is greatly appreciated.

Thank you,

Alex

Answer 1

What about:

If Not IsNumeric(txtNumberOne.Text) Or txtNumberOne.Text <= 0 Then
    MsgBox("Please Enter A Valid Number For Number One")
    txtNumberOne.Clear()
    Exit Sub
Else
    NumberOne = txtNumberOne.Text
End If

If Not IsNumeric(txtNumberTwo.Text) Or txtNumberTwo.Text < NumberOne Then
    MsgBox("Please Enter A Valid Number For Number Two")
    txtNumberTwo.Clear()
    Exit Sub
Else
    NumberTwo = txtNumberTwo.Text
End If

Be aware that if NumberOne is equal to Textbox2.Text, NumberTwo is never assigned

Answer 2

If Not IsNumeric(txtNumberOne.Text) Then 
    MsgBox("Please Enter A Valid Number For Number One") 
    txtNumberOne.Clear() 
    Exit Sub 
ElseIf txtNumberOne.Text = 0 Then 
    MsgBox("Please Enter A Valid Number For Number One") 
    txtNumberOne.Clear() 
    Exit Sub 
ElseIf txtNumberOne.Text > 0 And IsNumeric(txtNumberOne.Text) Then 
    NumberOne = txtNumberOne.Text 
End If

The third If is superflious. We know it must be IsNumeric, as it passed the first If, and we know it cannot be 0, as it passed the second If. (You also make no allowances at all, if it happens to be negative)

Now, it been a while since I last did VB.Net, but I'm pretty sure it still have a distinct between strings & integers, which means txtNumberOne.Text = 0 shouldn't even compile.

And also, why are you making your users guess what a "valid number" is?

Dim numberOne as Integer
If IsNumeric(txtNumberOne.Text) Then 
    numberOne = CInt(txtNumberOne.Text)
else
    numberOne = -1;
End If

If numberOne < 1
    MsgBox("Please Enter A Positive Number For Number One") 
    txtNumberOne.Clear() 
    Exit Sub 
End If

Answer 3

You should start with putting Option Strict On at the top of your code to force yourself to write code without implicit conversions between strings and numbers. An example of a pitfall in your code is where you compare the string value txtNumberTwo.Text to the numeric value NumberOne; it isn't obvious if the string is converted to a number so that the comparison works properly, or if the number is converted to a string so that it does a string comparison instead.

You can use the Int32.TryParse method to parse each number only once instead of three times:

Dim numberOne As Integer
Dim numberTwo As Integer
Dim result As Integer

If Not Int32.TryParse(txtNumberOne.Text, numberOne) Then
  MsgBox("Please Enter A Valid Number For Number One")
  txtNumberOne.Clear()
  Exit Sub
ElseIf numberOne <= 0 Then
  MsgBox("Please Enter A Valid Number For Number One")
  txtNumberOne.Clear()
  Exit Sub
End If

If Not Int32.TryParse(txtNumberTwo.Text, numberTwo) Then
    MsgBox("Please Enter A Valid Number For Number Two")
    txtNumberTwo.Clear()
    Exit Sub
ElseIf numberTwo < numberOne Then
    MsgBox("Please Enter A Valid Number For Number Two")
    txtNumberTwo.Clear()
    Exit Sub
End If

Your loop is not needed at all. You can calculate the sum directly:

Result = (numberOne + numberTwo) * (numberTwo + 1 - numberOne) / 2

Answer 4

i think the best solution for you is just set the textboxes to be able to accept only numbers, that way you can avoid all the checks if the texts is numeric or not and only check if its bigger than zero.

to set the textboxes to accept only numbers copy this function:

  Private Function TrapKey(ByVal KCode As String) As Boolean
    If (KCode >= 48 And KCode <= 57) Or KCode = 8 Then
        TrapKey = False
    Else
        TrapKey = True
    End If
  End Function

and in the keypress event of the textboxes add

  e.Handled = TrapKey(Asc(e.KeyChar))

Answer 5

    Dim doub As Double
    If Not (Double.TryParse(txtNumberOne.Text, doub) AndAlso doub > 0) Then
        MsgBox("Please Enter A Valid Number For Number One")

        txtNumberOne.Clear()
    Else
        NumberOne = doub
    End If


    If Not (Double.TryParse(txtNumberTwo.Text, doub) AndAlso doub > 0) Then
        MsgBox("Please Enter A Valid Number For Number Two")

        txtNumberTwo.Clear()
    Else
        NumberTwo = doub
    End If

I think this is waht you are looking for

  Result = (NumberTwo * (NumberTwo + 1) / 2) - ((NumberOne - 1) * (NumberOne) / 2)