3
\$\begingroup\$

Here is a simple calculator for calculating the tax and tip of a transaction. I have tried to add input verification. I am wondering if there are more elegant ways of doing some simple tasks that are required for this calculator, such as getting the total. Are my naming conventions correct, function use correct, variable (tax) convention correct (should it be static somewhere else?), and clearing/updating of fields as input is being entered into the form. Thank you!!

enter image description here

''' <summary>
''' Author:    Brandon Finley
''' Date:      10/5/2017
''' Purpose:   Calculate the tax and tip of a sale.
''' Status:    Works as intended. I tried to be as clear and consistant as possible.
''' </summary>

Public Class MainForum
    Dim tax As Double = 0.08

    ' Validates and updates tax and total boxes in dollar value.
    Private Sub CalcButton_Click(sender As Object, e As EventArgs) Handles CalcButton.Click
        If PriceBox.Text.Trim = "" Then
            ' If the PriceBox is empty, a value that isn't a number, or negative move cursor to PriceBox and give user reason.
            MsgBox("Enter a price.",
            MsgBoxStyle.Information, "Missing Information")
            PriceBox.Focus()
        ElseIf IsNumeric(PriceBox.Text) = False Then
            MsgBox("Must enter a number.",
            MsgBoxStyle.Information, "Verify")
            PriceBox.Focus()
        ElseIf PriceBox.Text < 0 Then
            MsgBox("Please enter a positive value into the Price field.",
            MsgBoxStyle.Information, "Verify")
            PriceBox.Focus()
        Else
            TaxBox.Text = Format(GetTax(), "currency")
            TotalBox.Text = Format(CalcTotal(), "currency")
        End If
    End Sub

    ' Returns the tax based on the tax multiplied by the price.
    Function GetTax() As Double
        Return PriceBox.Text * tax
    End Function

Private Function GetTip(PercentTip As GroupBox) As Double
    For Each ctrl As RadioButton In PercentTip.Controls
        If ctrl.Checked Then Return PriceBox.Text * (Microsoft.VisualBasic.Left(ctrl.Text, 2) / 100)
    Next
End Function

        ' Returns the total sales formatted to look like currency.
Private Function GetTotal(IsTipPercent As Boolean) As String
    ' Radio buttons do not return total until CalcButton has been pressed.
    If IsTipPercent = False Or (IsTipPercent = True And TotalBox.Text <> "") Then
        Return Format(CalcTotal(), "currency")
    End If
End Function

' Returns the total of price + tax + tip.
Private Function CalcTotal() As Double
    Return PriceBox.Text + GetTax() + GetTip()
End Function

' Updates price if tip amount changes (only after calculating total first).
' Updates price if tip amount changes (only after calculating total first).
Private Sub Tip15_CheckedChanged(sender As Object, e As EventArgs) Handles Tip15.CheckedChanged, Tip20.CheckedChanged, Tip25.CheckedChanged
    TotalBox.Text = GetTotal(True)
End Sub

    ' Clears the tax and tip fields if original price has been changed. bump
    Private Sub PriceBox_TextChanged(sender As Object, e As EventArgs) Handles PriceBox.TextChanged
        If TotalBox.Text <> "" And PriceBox.Text.Trim <> "" Then
            TaxBox.Text = ""
            TotalBox.Text = ""
        End If
    End Sub

    ' Exits application.
    Private Sub ExitButton_Click(sender As Object, e As EventArgs) Handles ExitButton.Click
        Me.Close()
    End Sub

End Class
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

My only concern would be the hard coding of 15, 20, and 25, and the redundant code used to handle these 3 values:

Private Function GetTip() As Double
    If (Tip15.Checked) Then
        Return PriceBox.Text * 0.15
    ElseIf (Tip20.Checked) Then
        Return PriceBox.Text * 0.2
    ElseIf (Tip25.Checked) Then
        Return PriceBox.Text * 0.25
    End If
End Function

[...]

Private Sub Tip15_CheckedChanged(sender As Object, e As EventArgs) Handles Tip15.CheckedChanged
    If TotalBox.Text <> "" Then
        TotalBox.Text = Format(CalcTotal(), "currency")
    End If
End Sub
Private Sub Tip20_CheckedChanged(sender As Object, e As EventArgs) Handles Tip20.CheckedChanged
    If TotalBox.Text <> "" Then
        TotalBox.Text = Format(CalcTotal(), "currency")
    End If
End Sub
Private Sub Tip25_CheckedChanged(sender As Object, e As EventArgs) Handles Tip25.CheckedChanged
    If TotalBox.Text <> "" Then
        TotalBox.Text = Format(CalcTotal(), "currency")
    End If
End Sub

You might want to add more values later (18% seems to be a new popular tip value for some reason) or edit the existing values, so hardcoding isn't a great idea, design/style wise.

\$\endgroup\$
4
  • \$\begingroup\$ Okay. Should I make the tip values global? Also, how can i reduce the redundancy of the tip*.CheckedChanged code. \$\endgroup\$
    – Brandon
    Commented Oct 5, 2017 at 16:19
  • \$\begingroup\$ Consider creating an array of Tip objects (class Tip with amount as an object variable). This seems more flexible to me. \$\endgroup\$
    – user1149
    Commented Oct 5, 2017 at 17:06
  • \$\begingroup\$ Alright, I added the function GetTotal that returns the total as a string to make Tip*_ChecedChanged code less redundant like you suggested. Still working on the hard coded tip % values. Edited the above code to show my new work. \$\endgroup\$
    – Brandon
    Commented Oct 5, 2017 at 22:23
  • \$\begingroup\$ I did what I could to fix the hard coded percent like you suggested. Now all you have to do is change the text in the radio button fields and the code will adapt. (code fixed in original post) \$\endgroup\$
    – Brandon
    Commented Oct 6, 2017 at 7:03

Not the answer you're looking for? Browse other questions tagged or ask your own question.