Efficiency of a Select Case

[Nandomo]

Is there any way I can make this select case more efficient?

Select Case cboBoard.SelectedIndex
            Case 0 To 1
                If intDaysRented = 0 Then
                    If intHoursRented > 3 Then
                        lblCost.Text = FormatCurrency(arrDailyRates(cboBoard.SelectedIndex), , TriState.True, TriState.True)
                    Else
                        lblCost.Text = FormatCurrency((arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented), , TriState.True, TriState.True)
                    End If
                Else
                    If intHoursRented > 3 Then
                        lblCost.Text = FormatCurrency((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented), , TriState.True, TriState.True)
                    Else
                        lblCost.Text = FormatCurrency(((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented) + (arrHourlyRates(0) * intHoursRented)), , TriState.True, TriState.True)
                    End If
                End If

            Case 2
                If intDaysRented = 0 Then
                    If intHoursRented > 2 Then
                        lblCost.Text = FormatCurrency(arrDailyRates(cboBoard.SelectedIndex), , TriState.True, TriState.True)
                    Else
                        lblCost.Text = FormatCurrency((arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented), , TriState.True, TriState.True)
                    End If
                Else
                    If intHoursRented > 2 Then
                        lblCost.Text = FormatCurrency((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented), , TriState.True, TriState.True)
                    Else
                        lblCost.Text = FormatCurrency(((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented) + (arrHourlyRates(2) * intHoursRented)), , TriState.True, TriState.True)
                    End If

                End If
            Case Else
                MsgBox("Something has gone terribly wrong with board selecting process.")
                End
        End Select

these would be the possible inputs that can be brought into the calculations. and in my combo box boogie is index 0 paddle 1 and surf 2

arrDailyRates(0) = 10  'for Boogie Board
        arrDailyRates(1) = 15  'for Paddle Board
        arrDailyRates(2) = 20  'for Surf Board

        arrHourlyRates(0) = 3  'for Boogie Board
        arrHourlyRates(1) = 5  'for Paddle Board
        arrHourlyRates(2) = 7  'for Surf Board

[djjeavons]

Hi

I tried out your code and one thing that struck me as not working quite right (although I don't know all of the rules) is that if you have the hours greater than 3 then you effectively get the item cheaper. For example, if I take the Boogie Board for 1 day and 3 hours it will cost £19.00 whereas if I take it for 1 day and 4 hours it will cost £10.00. Is this intentional?

If I was to assume that it is not and that you effectively charge both the day rate and the nuber of hours then the following code would be simpler:

    Dim hourlyRate As Decimal = 0D
    Dim dailyRate As Decimal = 0D

    'Ensure a selection has been made
    If cboBoard.SelectedIndex <= 2 Then
        hourlyRate = arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented
        dailyRate = arrDailyRates(cboBoard.SelectedIndex) * intDaysRented

        lblCost.Text = (dailyRate + hourlyRate).ToString("C")
    Else
        MessageBox.Show("Something has gone terribly wrong with board selecting process.")
    End If

Of course, this means that you could have hourly rates at any value, so you may want to do some sanitising on that first before performing the calculation. For example, you might want to say that if the hourlyRate exceeds 3 then give it to them on a day rate, so just increase the day rate by one and remove the hourly rate.

Another piece I noted is that you are using a lot of functions from the Microsoft.VisualBasic namespace such as MsgBox instead of MessageBox.Show and FormatCurrency instead of using .ToString and the currency conversion indicator. It is recommended not to use the VisualBasic namespace as this is more for assisting backwards compatibility (especially when upgrading VB6 projects to VB.NET).

HTH

[Nandomo]

Yeah I saw that too.

 Select Case cboBoard.SelectedIndex
            Case 0 To 1
                If intDaysRented = 0 Then
                    lblCost.Text = If(intHoursRented > 3, FormatCurrency(arrDailyRates(cboBoard.SelectedIndex), , TriState.True, TriState.True), FormatCurrency((arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented), , TriState.True, TriState.True))
                Else
                    lblCost.Text = If(intHoursRented > 3, FormatCurrency((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented), , TriState.True, TriState.True), FormatCurrency(((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented) + (arrHourlyRates(0) * intHoursRented)), , TriState.True, TriState.True))
                End If
            Case 2
                If intDaysRented = 0 Then
                    lblCost.Text = If(intHoursRented > 2, FormatCurrency(arrDailyRates(cboBoard.SelectedIndex), , TriState.True, TriState.True), FormatCurrency((arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented), , TriState.True, TriState.True))
                Else
                    lblCost.Text = If(intHoursRented > 2, FormatCurrency((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented), , TriState.True, TriState.True), FormatCurrency(((arrDailyRates(cboBoard.SelectedIndex) * intDaysRented) + (arrHourlyRates(2) * intHoursRented)), , TriState.True, TriState.True))
                End If
            Case Else
                MsgBox("Something has gone terribly wrong with board selecting process.")
                End
        End Select

I swapped out to this code.

[Reverend Jim]

I think your code would be a lot clearer, less prone to typos and easier to modify if you used intermediate variables. For example

DailyRate = arrDailyRates(cboBoard.SelectedIndex)

Select Case cboBoard.SelectedIndex

    Case 0 To 1

        If intDaysRented = 0 Then
            If intHoursRented > 3 Then
                cost = DailyRate
            Else
                cost = arrHourlyRates(cboBoard.SelectedIndex)
            End If              
        Else
            If intHoursRented > 3 Then
                cost = DailyRate * intDaysRented
            Else
                cost = DailyRate * intDaysRented + arrHourlyRates(0) * intHoursRented
            End If
        End If

    Case 2

        If intDaysRented = 0 Then
            If intHoursRented > 2 Then
                cost = DailyRate
            Else
                cost = arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented
            End If
        Else
            If intHoursRented > 2 Then
                cost  = DailyRate * intDaysRented
            Else
                cost = DailyRate * intDaysRented + arrHourlyRates(2) * intHoursRented
            End If
        End If

    Case Else

        MsgBox("Something has gone terribly wrong with board selecting process.")
        End

End Select

lblCost.Text = FormatCurrency(cost, , TriState.True, TriState.True)

[Nandomo]

You are right

Dim sngCost As Single
        Select Case cboBoard.SelectedIndex
            Case 0 To 1
                If intDaysRented = 0 Then
                    sngCost = If(intHoursRented > 3, arrDailyRates(cboBoard.SelectedIndex), (arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented))
                Else
                    sngCost = If(intHoursRented > 3, (arrDailyRates(cboBoard.SelectedIndex) * (intDaysRented + 1)), (arrDailyRates(cboBoard.SelectedIndex) * intDaysRented) + (arrHourlyRates(0) * intHoursRented))
                End If
            Case 2
                If intDaysRented = 0 Then
                    sngCost = If(intHoursRented > 2, arrDailyRates(cboBoard.SelectedIndex), (arrHourlyRates(cboBoard.SelectedIndex) * intHoursRented))
                Else
                    sngCost = If(intHoursRented > 2, (arrDailyRates(cboBoard.SelectedIndex) * (intDaysRented + 1)), (arrDailyRates(cboBoard.SelectedIndex) * intDaysRented) + (arrHourlyRates(2) * intHoursRented))
                End If
            Case Else
                MsgBox("Something has gone terribly wrong with board selecting process.")
                End
        End Select

        lblCost.Text = FormatCurrency(sngCost, , TriState.True, TriState.True)

[tinstaafl]

I would suggest some refactoring. A class(Board) can help keep the calculations separate from the event handler code, simplifying it tremendously:

Public Class Board
    Public type = ""
    Public hrRate = 0
    Public dayRate = 0
    Public hoursRented As Integer = 0
    Public daysRented As Integer = 0
    'This is returned as a decimal so that we can use the ToString
    'function to display it formatted and we can keep a running total
    Public Function Cost() As Decimal
        If (hoursRented > 3) Then
            daysRented += 1
            hoursRented = 0
        End If
        Return ((hoursRented * hrRate) + (daysRented * dayRate))
    End Function
    Public Overrides Function ToString() As String
        Return type
    End Function
End Class

Overriding the ToString() function allows us to use a List(Of Board) as the datasource for the combobox. The combobox will automatically use the ToString() function for its display. One advantage to this, is that the rates are automatically selected with the board type. This elimnates those concurrent arrays:

Public Sub New()

    ' This call is required by the designer.
    InitializeComponent()

    ' Add any initialization after the InitializeComponent() call.
    Dim boardList As New List(Of Board)(
        {
            New Board With {.type = "Boogie", .hrRate = 3, .dayRate = 10},
            New Board With {.type = "Paddle", .hrRate = 5, .dayRate = 15},
            New Board With {.type = "Surf", .hrRate = 7, .dayRate = 20}
        })
    cboBoard.DataSource = New BindingList(Of Board)(boardList)
End Sub

Now the selectedindexchanged handler can be coded without the complicated select-case block and the calculation code:

Private Sub cboBoard_SelectedIndexChanged(sender As Object, e As EventArgs) Handles cboBoard.SelectedIndexChanged
    Dim selectedBoard = DirectCast(cboBoard.SelectedItem, Board)
    selectedBoard.hoursRented = intHoursRented
    selectedBoard.daysRented = intDaysRented
    lblCost.Text = selectedBoard.Cost().ToString("C")
End Sub

One advantage to doing it this way, is expandability. It becomes fairly simple to add the capability for multiples of a single type of board and for multiple types of board

[Nandomo]

Lol I appreciate all that, I am going to have to read a little more before I can do stuff like that, you went a little too ham for me. But if you can give me a tutorial or something so I can learn stuff like that, it would be highly appreciated.

[tinstaafl]

Here's a tutorial on classes in vb.net

[Reverend Jim]

My advice is learn to walk before you learn to run. Get your code working simply before you complicate it with classes and overrides.