advise on simple error correcting calculator

[Rob Frank]

Hi I found this forum very helpful before in my java class and figured you guys could share some insight on my vb class. My professor  doesn't seem to share anything about any assignments being turned in so ya i can't determine if i'm really doing any of this stuff right. so this is the code that i came up with for a calories from fat calculator. it seems to work just fine and i have all my error messages working the way i want them to. It does seem rather drawn out to me. i'm not sure if it can be cleaned up and shortened but it is similar to other examples in the chapter i am on.

Public Class frmFatCalc
    Private Sub btnCalculate_Click(sender As Object, e As EventArgs) Handles btnCalculate.Click
        'declare variables
        Dim intCalories As Integer
        Dim intFat As Integer
        Dim decPercent As Decimal
        Const decFat_Calories As Integer = 9
        Dim blnInputOk As Boolean = True
        'error check and convert user input
        lblError.Text = String.Empty
        If Integer.TryParse(txtCalories.Text, intCalories) = False Then
            lblError.Text = "Calories must be an integer."
            txtCalories.Focus()
            blnInputOk = False
            Exit Sub
        End If
        lblError.Text = String.Empty
        If Integer.TryParse(txtFat.Text, intFat) = False Then
            lblError.Text = "Grams of fat must be an integer."
            txtFat.Focus()
            blnInputOk = False
            Exit Sub
        End If
        'error check that integers are greater than zero
        If Not intCalories > 0 Then
            lblError.Text = "Calories must be greater than zero."
            txtCalories.Focus()
            blnInputOk = False
            Exit Sub
        End If
        If Not intFat > 0 Then
            lblError.Text = "Grams of fat must be greater than zero."
            txtFat.Focus()
            blnInputOk = False
            Exit Sub
        End If
        'error check that calories from fat are not greater than total calories
        If intFat * decFat_Calories > intCalories Then
            MessageBox.Show("Calories from fat cannot be greater than total calories.
Please check the label and input the correct values.")
            blnInputOk = False
            Exit Sub
        End If
        'calculate calories from fat percentage
        If blnInputOk = True Then
            decPercent = CDec((intFat * decFat_Calories) / intCalories)
            lblPercent.Text = decPercent.ToString("p")
        End If
        'display message if food is low in fat
        If decPercent < 0.3D Then
            lblError.Text = "This food is low in Fat"
        End If
    End Sub

    Private Sub btnClear_Click(sender As Object, e As EventArgs) Handles btnClear.Click
        'clear text input fields
        txtCalories.Clear()
        txtFat.Clear()
        'clear all labels
        lblError.Text = String.Empty
        lblPercent.Text = String.Empty
        'set cursor back to first field after hitting clear button
        txtCalories.Focus()
    End Sub

    Private Sub btnExit_Click(sender As Object, e As EventArgs) Handles btnExit.Click
        'closes form
        Me.Close()
    End Sub

[Rob Frank]

bump.

I guess vb isn't as popular as java lol.

[Pete Letkeman]

Your are correct, many more Java minded people on this site then VB people.
Can you provide more information such as:
What version of VB?
What errors messages are you experiencing?
What steps are required to reproduce the incorrect result?

[Rob Frank]

i'm using visual studio 2015 enterprise. I have no errors or trouble. I was simply looking for someone with experience to tell me if i need to improve on anything in my code to possibly make it cleaner or more simple.

[Pete Letkeman]

I do see a few things which I think could be changed to make your code clearer.
I'm not too sure that you should compare values like thisIf Integer.TryParse(txtCalories.Text, intCalories) = False Then    lblError.Text = "Calories must be an integer."    txtCalories.Focus()    blnInputOk = False    Exit Sub End IfWould this be easier:If Not (Integer.TryParse(txtCalories.Text, intCalories)) Then    lblError.Text = "Calories must be an integer."    txtCalories.Focus()    blnInputOk = False    Exit Sub End If
You have a number of if statements which validate input and exit the function if needed.
Would you be able to combine then with OR or AND or nested Ifs to create something kind of like this
If Integer.TryParse(txtCalories.Text, intCalories) Then if (intCalories > 0) Then blnInputOk = True Else lblError.Text = "Calories must be an integer value greater then zero." txtCalories.Focus() blnInputOk = False End If End IfOn line 17 you have repeated this statementlblError.Text = String.EmptyAre you sure that you need it?
Do you think that brackets may help out in the reading of this line If intFat * decFat_Calories > intCalories Then to make it If (intFat * decFat_Calories) > intCalories Then
I noticed that you have a number of "Exit Sub" lines....Personally I'm not a fan of that, but that is my own opinion.
I think it may be better to have your "Const" lines before your "Dim" lines.

[Rob Frank]

Ok thanks. I appreciate it. I made some of the changes you recommended. I kept the integer and the zero errors separate only because i think the problem in the book wants both error messages. Not sure if it makes a difference or not in my grade. just playing that safe. I added the exit subs in last. I found some input online searching when i couldn't get my error messages to stop. I was a little confused because I did if statements in java and didn't have a problem and when i did if, else if and else it seemed to play through the whole thing instead of stopping at the first error message. I see you say its just your opinion not to use exit sub so can i ask what your preferred method is? I don't have very many opinions on any of this since it is still all very new to me.
Thanks,
Rob

[Pete Letkeman]

A couple of postings are due now:
Note: This is code for Visual Studio 2017 (community/free) and not Visual Studio 2015
Here is the C# code for the main button which may or may not help you along:
private void Button_Click(object sender, RoutedEventArgs e) { const int decFat_Calories = 9; // declare variables int intCalories = 0; int intFat = 0; Decimal decPercent = 0; bool blnInputOk = true; lblError.Content = String.Empty; if (!(int.TryParse(txtCalories.Text, out intCalories))) { lblError.Content = "Calories must be an integer."; txtCalories.Focus(); blnInputOk = false; } if (blnInputOk) { if (!(int.TryParse(txtFat.Text, out intFat))) { lblError.Content = "Grams of fat must be an integer."; txtFat.Focus(); blnInputOk = false; } } if (blnInputOk) { // error check that integers are greater than zero if (!(intCalories > 0)) { lblError.Content = "Calories must be greater than zero."; txtCalories.Focus(); blnInputOk = false; } } if (blnInputOk) { if (!(intFat > 0)) { lblError.Content = "Grams of fat must be greater than zero."; txtFat.Focus(); blnInputOk = false; } } if (blnInputOk) { // error check that calories from fat are not greater than total calories if (((intFat * decFat_Calories) > intCalories)) { MessageBox.Show("Calories from fat cannot be greater than total calories"); } } }
The IDE states that the variable decPercent is never used. If this is truly the case then you can remove it.
Here is the VB.NET (2017) code (created using free online C# to VB.NET converter):
Private Sub Button_Click(ByVal sender As Object, ByVal e As RoutedEventArgs) Const decFat_Calories As Integer = 9 ' declare variables Dim intCalories As Integer = 0 Dim intFat As Integer = 0 Dim decPercent As Decimal = 0 Dim blnInputOk As Boolean = true lblError.Content = String.Empty If Not Integer.TryParse(txtCalories.Text, intCalories) Then lblError.Content = "Calories must be an integer." txtCalories.Focus blnInputOk = false End If         If blnInputOk Then If Not Integer.TryParse(txtFat.Text, intFat) Then lblError.Content = "Grams of fat must be an integer." txtFat.Focus blnInputOk = false End If             End If         If blnInputOk Then ' error check that integers are greater than zero If Not (intCalories > 0) Then lblError.Content = "Calories must be greater than zero."                 txtCalories.Focus blnInputOk = false End If             End If         If blnInputOk Then If Not (intFat > 0) Then lblError.Content = "Grams of fat must be greater than zero." txtFat.Focus blnInputOk = false End If             End If         If blnInputOk Then ' error check that calories from fat are not greater than total calories If ((intFat * decFat_Calories)  > intCalories) Then MessageBox.Show("Calories from fat cannot be greater than total calories") End If End If End Sub
I suspect that you have to change the .Content to .Text for the labels code so that works in 2015 versions.

[Pete Letkeman]

Rob Frank wrote:I see you say its just your opinion not to use exit sub so can i ask what your preferred method is?

I like to use a control variable, which you are setting you called it blnInputOk. The case for having many 'exit sub' lines is the case for having many return lines in Java.
Both are perfectly valid and used by many different programmers. I suggest you pick a style/preference and stick with it when working on a project.

If you look at the code that I created you see that I wrapped an if statement around many of the blocks.
I could nest the if statements more to do something like:
If Not Integer.TryParse(txtCalories.Text, intCalories) Then lblError.Content = "Calories must be an integer." txtCalories.Focus ElseIf Not Integer.TryParse(txtFat.Text, intFat) Then lblError.Content = "Grams of fat must be an integer." txtFat.Focus Else ' error check that integers are greater than zero If Not (intCalories > 0) Then lblError.Content = "Calories must be greater than zero." txtCalories.Focus ElseIf Not (intFat > 0) Then lblError.Content = "Grams of fat must be greater than zero." txtFat.Focus Else ' error check that calories from fat are not greater than total calories If ((intFat * decFat_Calories)  > intCalories) Then MessageBox.Show("Calories from fat cannot be greater than total calories") End If End If End IfWhich may or may not be a better solution or easier to read. With this, are not use the blnInputOk variable.

In the end the question that you have to ask yourself is "Which style do I think is easier to follow?".
The one that you think is easier to follow is the one that you should use.
Eventually you will most likely have revisit your code and if it's easy to follow then it will be easier to update.

[Pete Letkeman]

One more piece of advice if I may.
There is a rich set of addons/plugins for Visual Studio. Some of these are free, some are not free.

I suggest that you take a look at the follow two (among the many others out there):
- CodeMaid https://marketplace.visualstudio.com/items?itemName=SteveCadwallader.CodeMaid
- StyleCop (many to choose from) https://marketplace.visualstudio.com/search?term=stylecop&target=VS&category=All%20categories&vsVersion=&sortBy=Relevance

These two will help you keep your code clean, formatted correctly, suggest removal of unused variables, organize your logic and more.
They are 100% free and can be installed on Visual Studio 2017 (all editions) and most (all?) editions of Visual Studio 2015.

Note that, however unlikely, any addons/extras could make Visual Studio use more memory and may decrease the stability of Visual Studio.

[Rob Frank]

A lot of good advise thankyou. I will  check into those addons. Visual studio has a lot of great features and the only issue  I found with the 2015 edition is that the undo button doesn't  always light up. From what I can tell it's a known issue. I really do like the suggestions it gives when I code wrong really saves me from pounding my head like I did with Java using netbeans. But if these addons will go past what what VS already does that would be awesome.