New VB'er here (help please!)

[CrisisV]

Hey guys, I am working on a project for school. We have to re-engineer some code my professor gave us and fix the issues and comment it and such. I think I've gotten a handle on most of it however, it states that "X" has to be a number less than 10. When I run the program and enter a number more than 10 the program still runs so I need a way to limit it at 10, yet whenever I write a new If Then statement I get a stack overflow and infinite looping and it crashes. Can anyone give me any help or advise me how to fix this? I'm going to keep working on it.

Thanks!

BTW Here is the Code I am working on

If x <= 1 Then
            Factorial = 1
        Else
            Factorial = x * Factorial(x - 1)
        End If

If x > 10 Then
            Factorial = "Pick a number less than 10" 
        Else
            Factorial = x * Factorial(x - 1)
        End If

    End Function

[GaryMazzone]

Try like this:

If x <= 1 Then
     Factorial = 1
ElseIf x>1 ANDALSO x < 10
     Factorial = x * Factorial(x - 1)
Else
     Factorial = "Pick a number less than 10" 
End If

[Hack]

There is no looping going on with the code you posted. Typically a Stack Overflow will be caused by a recursive loop gone wild....there has to be more to the code that what you have shown.

But, the thing that really jumps out at me is what are you using the same variable for a number and a string? I guess that means Factorial is declared As Variant, right?

[CrisisV]

To GarryMazzone I will try that out and write back. Thanks.

[CrisisV]

Alright, well I have tried that out..and after some declaration problems I have fixed the issue and all seems well now! Thanks GarryMazzone! I had "X" originally declared as Double and that was throwing everything off. Now "X" is declared as Object and can properly display the results and the error message. Thanks!

[GaryMazzone]

Why would declare x as an Object. Interger is fine for this. The problem is probably how you get the value of x. It would help to see that.

[CrisisV]

Ok, I will post the whole code. It's not actually that long I just didn't want to make everything crazy looking if I didn't have to. This is what I have as of right now.

I declared "X" as Object because I was getting StackOverflow errors with the conversion to the String message (that you helped me with before). This made it work. Is this not a very efficient way to go about things?

Public Class Form1

    Private Sub btnFactorial_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnFactorial.Click
        
        Dim x As Object
        Dim Fact As Object

        lblOutput.Text = "To determine the Factorial of a number. " _
            & "Please enter a number less than 10." _
            & "Then, click on the Factorial Button to view the results."
       
        x = Val(txtInput.Text)

        Fact = Factorial(x)
        lblOutput.Text = ("")
        lblOutput.Text = ("       " & x & " is   " & Fact)

    End Sub

    Private Function Factorial(ByRef x As Double) As Object

        If x <= 1 Then
            Factorial = 1
        ElseIf x > 1 AndAlso x < 10 Then
            Factorial = x * Factorial(x - 1)
        Else
            Factorial = "Please pick a number less than 10"
        End If

    End Function

End Class

[GaryMazzone]

The function should be changed like this:

  Private Function Factorial(ByRef x As Integer) As Integer

        If x <= 1 Then
            Factorial = 1
        ElseIf x > 1 AndAlso x < 10 Then
            Factorial = x * Factorial(x - 1)
        Else
            Factorial = "Please pick a number less than 10"
        End If

    End Function

The code in the button click should be:

Dim x As Integer       
Dim Fact As Inetger

        lblOutput.Text = "To determine the Factorial of a number. " _
            & "Please enter a number less than 10." _
            & "Then, click on the Factorial Button to view the results."
       
If NOT Inetger.TryParse(Me.txtInput.Text,x) Then
     MessageBox.Show("Please enter a number from 1 to 10 only")
     Me.txtInput.Focus()
     Exit Sub
End If
If x < 10 then 
   MessageBox.Show("Only values less then 10 are allowed here.")
   Me.txtInput.Focus()
   Exit Sub
End If
        Fact = Factorial(x)
        lblOutput.Text = ("")
        lblOutput.Text = ("       " & x & " is   " & Fact)

[akhileshbc]

Here's a bit cleaned version:

Public Class Form1
 
    Private Sub btnFactorial_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnFactorial.Click
 
        Dim x As Integer
        Dim Fact As Integer
 
        '~~~ Check if the we are successful in converting the number entered in textbox to a valid integer. If not, display a message. If successful, "x" would contain the converted Integer value
        If Not Integer.TryParse(txtInput.Text, x) Then
            lblOutput.Text = "To determine the Factorial of a number. " _
            & "Please enter a number less than 10." _
            & "Then, click on the Factorial Button to view the results."
        End If
 
        '~~~ Calculate the factorial of "x" and store it in "Fact"
        Fact = Factorial(x)
 
        '~~~ If the number is not >1 and <10, then the function "Factorial" would return "-1". So, we are checking it here. If so, we'll output a message. Otherwise, display the result of the factorial
        If Fact <> -1 Then
            lblOutput.Text = ("       " & x.ToString & " is   " & Fact.ToString)
        Else
            lblOutput.Text = "Please pick a number less than 10"
        End If
 
    End Sub
 
    Private Function Factorial(ByRef x As Integer) As Integer
 
        '~~~ This is an exit condition for the recursive call.
        If x = 1 Then
            Factorial = 1
        ElseIf x > 1 AndAlso x < 10 Then
            Factorial = x * Factorial(x - 1)
        Else
            Factorial = -1
        End If
 
    End Function
End Class

[GaryMazzone]

Thanks for the catch on the function. I did it on-line and never tested

[Hack]

If you add "Option Strict On" to your project, you will get a number of errors immediately returned with what you have.

On the other hand, this worked just fine for me

Public Class Form1
    Private Function Factorial(ByRef x As Double) As Double
        Dim strFact As String
        If x <= 1 Then
            Factorial = 1
        ElseIf x > 1 AndAlso x < 10 Then
            Factorial = x * Factorial(x - 1)
        Else
            strFact = "Please pick a number less than 10"
            lblOutput.Text = strFact
        End If
 
    End Function
 
    Private Sub btnFactorial_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnFactorial.Click
        Dim x As Double
        Dim Fact As Double
 
        lblOutput.Text = "To determine the Factorial of a number. " _
            & "Please enter a number less than 10." _
            & "Then, click on the Factorial Button to view the results."
 
        x = Val(txtInput.Text)
 
        Fact = Factorial(x)
        lblOutput.Text = ("")
        lblOutput.Text = ("       " & x & " is   " & Fact)
 
    End Sub
End Class

[CrisisV]

Thanks guys! Will look over and re-edit my stuff. Ya'll rock!

[Evil_Giraffe]

Everyone's Factorial functions are wrong. They would only be correct if the function was named FactorialOfANumberLessThanTen. The check that the input is less than ten does not belong in the Factorial function.

Verify the input is less than ten, then call Factorial.

And on the subject of the Factorial function... are you aware that this is a concept called recursion? If you were familiar with it then you wouldn't be surprised that a mistype can end up with a StackOverflow exception. When calling itself, a function must be one step closer to the terminating condition (in this case, when you ask for the factorial of 1).

Recursive functions are always in danger of getting a stack overflow even when coded correctly, if the initial arguments require too many recursions to solve the problem. To avoid this, you can sometimes make the recursive function a tail-recursive function. This means that the recursive call is the last thing the function does and it immediately returns after making it. In our current factorial example, the function looks like this (having stripped it down to just the factorial calculation):

Public Function Factorial(ByVal x As Integer) As Integer
    If x <= 1
        Factorial = 1
    Else
        Factorial = x * Factorial(x - 1)
    End If
End Function

This is not tail-recursive. After making the recursive call, the function multiplies the returned value by x before returning. However, consider the following alteration, where we introduce an accumulator:

Public Function Factorial2(x As Integer) As Integer
    Return Factorial2Impl(x, 1)
End Function
 
Private Function Factorial2Impl(x As Integer, accumulator As Integer) As Integer
    If x <= 1
        Factorial2Impl = accumulator
    Else
        Factorial2Impl = Factorial2Impl(x - 1, x * accumulator)
    End If
End Function

You can see that in this version, the FactorialImpl function returns the result of the recursive call immediately. This version is tail-recursive. The multiplication happens before making that recursive call and the result is passed down in the accumulator.

Now, why is tail-recursion useful? It would seem that you've got as many recursive calls as the first version. The advantage comes because it is possible to perform optimisation on a tail-recursive function to turn it into an iterative function, thereby not consuming stack frames. This optimisation can be done by a compiler automatically (if it is sufficiently smart - not all compilers will be able to do this, and you normally need to explicitly tell those that can that they should)!

Public Function Factorial3(x As Integer) As Integer
    Return Factorial3Impl(x, 1)
End Function
 
Private Function Factorial3Impl(x As Integer, accumulator As Integer) As Integer
    While x > 1
        accumulator *= x
        x -= 1
    End While
    Factorial3Impl = accumulator
End Function

And thus, at a stroke, your tail-recursive function is rewritten as an iterative function. Another optimisation that will often be possible is to inline the second function:

Public Function Factorial4(x As Integer) As Integer
    Dim accumulator As Integer = 1
    While x > 1
        accumulator *= x
        x -= 1
    End While
    Factorial4 = accumulator
End Function

[CrisisV]

Thanks again everyone, just got out of class and this is what my final lab looked like. I appreciate all the help.

Public Class Form1

    Private Sub btnFactorial_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnFactorial.Click
       
        Dim userinput As Double ' Declares userinput as the Double data type :D
        Dim Fact As Double ' Declares Fact as the Double data type ..... haha

        ' This line takes the number that is inside the Textbox and puts it as userinput
        userinput = Val(txtInput.Text)

        ' This line changes the direction text and sets it to the results of the calculation of the Factorial function. 
        Fact = Factorial(userinput)
        lblOutput.Text = ("")
        lblOutput.Text = ("       " & userinput & " is   " & Fact)

        ' This runs an error check to make sure the user inputed a number between 1 and 10. 

        If userinput < 1 Then
            lblOutput.Text = "Please pick a number between 1 and 10."
        ElseIf userinput > 10 Then
            lblOutput.Text = "Please pick a number between 1 and 10."
        End If

    End Sub

    Private Function Factorial(ByRef userinput As Double) As Double

        ' This code is the actual Factorial math. Basically what this does is take the userinput whatever it may be
        ' and starts with that (i.e. 5) and then multiply it by each number down until you reach 1 (i.e. 5*4*3*2*1)
        ' Awesome. 

        If userinput <= 1 Then
            Factorial = 1
        Else
            Factorial = userinput * Factorial(userinput - 1)
        End If

    End Function


    Private Sub Form1_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load

        ' Below is just the text that gives the user instructions on how to operate the program. This is loaded
        ' at startup. 
        lblOutput.Text = "To determine the Factorial of a number. " & Chr(13) _
            & "Please enter a number less than 10." & Chr(13) _
            & "Then, click on the Factorial Button to view the results."

    End Sub

End Class

[Evil_Giraffe]

        Dim userinput As Double ' Declares userinput as the Double data type :D
        Dim Fact As Double ' Declares Fact as the Double data type ..... haha

I'm glad that declaring variables causes you so much pleasure???

[CrisisV]

my professor told us not to do that since it was actually extremely redundant to comment exactly what the code was doing. I felt like being a smart ass. Haha.

[Evil_Giraffe]

Your professor has a very good point. If you stick with programming, then in about 10 or 20 years time you'll understand why Until then, just follow his advice, ok?

[CrisisV]

Of course Evil. I just know she appreciates humor as well