[2003] Can someone help review this code? [Resolved]

[Poppeseed]

I'm just learning Visual Basic .net, taking a class at the local acommunity college, so give me a break . Here was the assignment "Create an application that allows the user to enter the gender (either F or M) and GPA for one or more students. The application should calculate the average GPA for all students, the average GPA for male students, and the average GPA for female students." (Note, the below code is in a button click). When I click the button after inserting correct info into the text boxes, it just puts that info directly into the male or female label, and never increases/accumulates/whatever in the average GPA labels. Any suggestions? What am I doing wrong?
Thanks, Poppe.

        'declare variables and constants
        Dim strGender As String
        Dim decGPA As Decimal
        Dim intNumMale As Integer 'counter
        Dim intNumFemale As Integer 'counter
        Dim decGPAMale As Decimal
        Dim decGPAFemale As Decimal
        Dim decGPAMaleTotal As Decimal 'accumulator
        Dim decGPAFemaleTotal As Decimal 'accumulator
        Dim DecGPATotal As Decimal

        'determine if txtGender is M or F and if txtGPA is numeric
        If Me.txtGender.Text.ToUpper() = "M" Or Me.txtGender.Text.ToUpper() = "F" Then
            'assign input values to variables
            decGPA = Convert.ToDecimal(Me.txtGPA.Text)
            strGender = Me.txtGender.Text.ToUpper()

            'determine if decGPA is between 0 and 4
            If decGPA >= 0D AndAlso decGPA <= 4D Then

                If "M" = strGender Then
                    'calculate average Male GPA
                    intNumMale = intNumMale + 1
                    decGPAMaleTotal = decGPAMaleTotal + decGPA
                    decGPAMale = decGPAMaleTotal / intNumMale
                Else
                    'calculate average Female GPA
                    intNumFemale = intNumFemale + 1
                    decGPAFemaleTotal = decGPAFemaleTotal + decGPA
                    decGPAFemale = decGPAFemaleTotal / intNumFemale
                End If
                'calculate average total GPA
                DecGPATotal = (decGPAMale + decGPAFemale) / 2

                'display GPA's
                Me.lblMale.Text = decGPAMale.ToString("F2")
                Me.lblFemale.Text = decGPAFemale.ToString("F2")
                Me.lblTotal.Text = DecGPATotal.ToString("F2")
            Else
                MessageBox.Show("The GPA must be between 0 and 4.", _
                "GPA Calculator", MessageBoxButtons.OK, MessageBoxIcon.Error, _
MessageBoxDefaultButton.Button1)
            End If
        Else
            MessageBox.Show("The gender must be either M or F", _
                "GPA Calculator", MessageBoxButtons.OK, MessageBoxIcon.Error, _
MessageBoxDefaultButton.Button1)
        End If

[Atheist]

Welcome to the forums
Ok heres a couple of things I would change:

This:

If Me.txtGender.Text.ToUpper() = "M" Or Me.txtGender.Text.ToUpper() = "F" Then

To this:

If Me.txtGender.Text.Equals("M", StringComparison.CurrentCultureIgnoreCase) Or Me.txtGender.Text.Equals("F", StringComparison.CurrentCultureIgnoreCase) Then

Simply because I consider it 'ugly' to make something uppercase before comparing it.

This:

decGPA = Convert.ToDecimal(Me.txtGPA.Text)

To this:

If Not decimal.TryParse(Me.txtGPA.Text, decGPA) Then
    MessageBox.Show("Enter a valid decimal in txtGPA")
    Return
End If

Because, if you dont, your application will throw an exception if the textbox contains something that cant be converted to a decimal value. Using the code i provided will convert the text to a decimal value and put it in the desired variable, if it fails it will show the messagebox and exit the subroutine. Better than crashing eh?

This:

intNumMale = intNumMale + 1

To this:

intNumMale += 1

No big thing, I just think it makes the code more readable.

Other than that, its actually well done.

[Poppeseed]

Thanks, but when I do those changes, it gives the errors:
1. Name 'StringComparison' is not declared.
2. Name 'StringComparison' is not declared.
3. TryParse is not a member of Decimal.

I'm running 2003, if that matters.
Also, will this fix the problem with the running totals?

[Atheist]

Thanks, but when I do those changes, it gives the errors:
1. Name 'StringComparison' is not declared.
2. Name 'StringComparison' is not declared.
3. TryParse is not a member of Decimal.

I'm running 2003, if that matters.
Also, will this fix the problem with the running totals?

Ooh im sorry I seem to have missed you where using 2003, that changes things. Forget what Ive said (If you dont feel like upgrading to 2005 express, its free )

Im going to take a closer look at your code to spot what can be causing your problems.

[Atheist]

Are all the declarations declared inside the subroutine? Or are any of them declared at class level?

[Poppeseed]

Ooh im sorry I seem to have missed you where using 2003, that changes things. Forget what Ive said (If you dont feel like upgrading to 2005 express, its free )

Im going to take a closer look at your code to spot what can be causing your problems.

Thank you. I will probably upgrade after my class is over, but 2003 is required for the class.

Are all the declarations declared inside the subroutine? Or are any of them declared at class level?

They are all inside the subroutine. Would putting them at the class level fix this?

[Atheist]

When you declare a variable inside a subroutine, it will be reset everytime the subroutine is called IF theyre not declared with the Static keyword.
I suppose there are some variables in there that you dont want to reset for each button click (the male and female count variables for instance).

So you have two options, either declare them using the static keyword or declare them outside the procedure.

[Poppeseed]

Well, since I haven't learned about static yet, it would probably be best just to put the ones that need to not be reset at the class level. Thank you for your help, the application seems to work like its supposed to.