Average Pace Calculator

[junkie_ball]

Hi,

This is my first post here (of many i hope). I been dabbling with VB on and off for a few years now i started with VB6 and now using VB.Net. I'm trying to generate a simple average pace calculator. I have coded the calculator to a stage where it seems to work although on testing it i have noted my average pace rounding seems to vary slightly from others online. Although i have also note different calculators online seem to vary from each other.

I have used the .tostring("0.##") as a means of converting my calculation back to actual time rather than a decimal. Is this the correct method or is there a better way?

I have posted the code below. Please excuse it if the code is not as neat as it could be but like i start i'm still a novice.

        Dim Hours, Mins, Secs As Integer
        Dim Distance, Pace As Decimal


        Try
            Distance = CDec(txtDistance.Text)
            'Ensures all time fields have data
            If txtHrs.Text = "" Or txtMins.Text = "" Or txtSecs.Text = "" Then
                MsgBox("Please enter a time to calculate average pace from", MsgBoxStyle.Information, "Average Pace Calculator")
            Else
                Try
                    Hours = CInt(txtHrs.Text)
                    Mins = CInt(txtMins.Text)
                    Secs = CInt(txtSecs.Text)

                    If cmbDistanceUnit.Text = "Mile" Then
                        Pace = ((Hours * 60) + (Mins) + (Secs / 60)) / Distance
                        txtPace.Text = Pace.ToString("0.##") & " Per Mile"
                    ElseIf cmbDistanceUnit.Text = "Kilometer" Then
                        Pace = ((Hours * 60) + (Mins) + (Secs / 60)) / Distance
                        txtPace.Text = Pace.ToString("0.##") & " Per Kilometer"
                    Else
                        MsgBox("Please Select Kilometers or Miles", MsgBoxStyle.Information, "Average Pace Calculator")
                    End If

                Catch ex As Exception
                    MsgBox("Please enter a valid time", MsgBoxStyle.Information, "Average Pace Calculator")
                End Try
            End If
        Catch ex As Exception
            MsgBox("Please enter a valid distance", MsgBoxStyle.Information, "Average Pace Calculator")
        End Try

 

[jmcilhinney]

The first point to note is that it's basd practice to force a conversion and catch an exception if it failes if you don't have to. Rather than using CDec and CInt, you should be using TryParse, e.g.

Dim distance As Decimal

If Decimal.TryParse(distanceTextBox.Text, distance) Then
    '...
Else
    'Invalid distance.
End If

Also, to get the number of minutes you should use a TimeSpan, e.g.

Dim time As New TimeSpan(hours, minutes, seconds)
Dim minutes As Double = time.TotalMinutes

 

[junkie_ball]

Also, to get the number of minutes you should use a TimeSpan, e.g.

Dim time As New TimeSpan(hours, minutes, seconds)
Dim minutes As Double = time.TotalMinutes

Thanks for the advice i need all i can get! I have tried re-writing my code using your examples above. I was expecting the total minutes in the time varible (I.E. Hours + Minutes + Seconds) to be placed in the variable minutes with the declarion line of 'Dim TotalMinutes As Double = time.TotalMinutes' is this not correct?

I have the correct values being passed to the (hours, Minutes, Secs) and have confirmed this by stepping through the code although the value of varible TotalMinutes still remains at 0.0

        Dim Hours, Minutes, Secs As Integer
        Dim Time As New TimeSpan(Hours, Minutes, Secs)
        Dim TotalMinutes As Double = Time.TotalMinutes

        Integer.TryParse(txtHrs.Text, Hours)
        Integer.TryParse(txtMins.Text, Minutes)
        Integer.TryParse(txtSecs.Text, Secs)

Apologise for the basic question here but all novices have to start somewhere hey!

 

[jmcilhinney]

The problem is that you're creating the TimeSpan before you assign values to Hours, Minutes and Secs. As such, they are all 0 when the TimeSpan is created so of course the TotalMinutes is 0.

Also, TryParse should generally be used in an If statement, as I've demonstrated. It returns True or False to indicate whether the input was valid. As you have it, if the input is invalid you will just use 0 for that component. If that's what you want then that's fine but it's generally better to notify the user of invalid input so that they can fix it and try again.

 

[junkie_ball]

THanks again for the advice will play around with the suggestions. I do have tryparse in an if statement just pulled them out when I was testing to see if I could get the timespan function to opperate correctly.

Great advice as my code looks far neater already without the try catch statements appreciate the help.

 

[junkie_ball]

Ok have now had time to retry the code as suggested and seems to work without problems. I have now also introduced a function to avoid code repetition for the average pace calculation. Really appreciate the advice and guidance in this post.

Going back to my initial enquiry i still seem to be ending up with slightly different average pace than other calculators online. If this was only a second i could put this down to rounding of data but at times this can be up to 3 seconds difference on the average?

    Private Sub btnPace_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnPace.Click

        Dim Hours, Minutes, Secs As Integer
        Dim Distance As Decimal

        If Decimal.TryParse(txtDistance.Text, Distance) Then
            If cmbDistanceUnit.Text = "Mile" Then
                If Integer.TryParse(txtHrs.Text, Hours) And Integer.TryParse(txtMins.Text, Minutes) And Integer.TryParse(txtSecs.Text, Secs) Then
                    txtPace.Text = PaceCalc(Hours, Minutes, Secs, Distance).ToString("0.##") & " Minutes Per Mile"
                Else
                    MsgBox("Please enter a valid time", MsgBoxStyle.Information, "Average Pace Calculator")
                End If
            ElseIf cmbDistanceUnit.Text = "Kilometer" Then
                If Integer.TryParse(txtHrs.Text, Hours) And Integer.TryParse(txtMins.Text, Minutes) And Integer.TryParse(txtSecs.Text, Secs) Then
                    txtPace.Text = PaceCalc(Hours, Minutes, Secs, Distance).ToString("0.##") & " Minutes Per Kilometer"
                End If
            Else
                MsgBox("Please select Kilometer or Mile", MsgBoxStyle.Information, "Average Pace Calculator")
            End If
        Else
            MsgBox("Please enter a valid distance", MsgBoxStyle.Information, "Average Pace Calculator")
        End If

    End Sub

    Private Function PaceCalc(ByVal Hours As Integer, ByVal Minutes As Integer, ByVal Seconds As Integer, ByVal Distance As Decimal) As Decimal

        Dim Time As New TimeSpan(Hours, Minutes, Seconds)
        Dim TotalMinutes As Double = Time.TotalMinutes
        PaceCalc = TotalMinutes / Distance

    End Function

 

[jmcilhinney]

First up, on the subject of code repetition, you still have quite a bit in there. You've got this twice:

If Integer.TryParse(txtHrs.Text, Hours) And Integer.TryParse(txtMins.Text, Minutes) And Integer.TryParse(txtSecs.Text, Secs) Then

plus you've got these both:

txtPace.Text = PaceCalc(Hours, Minutes, Secs, Distance).ToString("0.##") & " Minutes Per Mile"

txtPace.Text = PaceCalc(Hours, Minutes, Secs, Distance).ToString("0.##") & " Minutes Per Kilometer"

which differ by a single word. This type of code duplication can lead to errors... and you have one in your code. If the user selects "Mile" but doesn't enter a valid time you are notifying them, but you aren't if they select "Kilometer". You should be parsing time once and once only because that's the same irrespective of the unit they select. Also, there's no need to have sepasrate code for the two units because what you do in each case is exactly the same except for the fact that the unit is on the end of the output. In that case, what you should do is put the unit on the end of the output:

Dim Hours, Minutes, Secs As Integer
Dim Distance As Decimal

If Not Decimal.TryParse(txtDistance.Text, Distance) Then
    'Invalid distance.
ElseIf cmbDistanceUnit.SelectedIndex = -1 Then
    'Invalid unit.
ElseIf Not Integer.TryParse(txtHrs.Text, Hours) OrElse _
       Not Integer.TryParse(txtMins.Text, Minutes) OrElse _
       Not Integer.TryParse(txtSecs.Text, Secs) Then
    'Invalid time.
Else
    txtPace.Text = String.Format("{0:0.##} Minutes Per {1}", _
                                 PaceCalc(Hours, Minutes, Secs, Distance), _
                                 cmbDistanceUnit.Text)
End If

Nicer, right?

As for the calculation, have you actually checked the result by hand? If you put the same values into a calculator, what value do you get? Is it significantly different to what your code produces?

 

[junkie_ball]

Wow I have so much to learn about tidying my code up to minimise my errors! Your way is far shorter and easier to understand.

Have thrown another slight query up for example if i run the program and use the following values it returns an average pace of 7.66 Per Minute when i would expect it to be 5.39 per minute. Perhaps i'm miss understanding the timespan function as i thought the output should be converted to minutes and seconds? Even if my equation is incorrect which i'm currently checking .66 should not be a value thats returned?

Values

Distance = 16
Time = 1hr 30mins 30secs

 

[jmcilhinney]

Wow I have so much to learn about tidying my code up to minimise my errors! Your way is far shorter and easier to understand.

Have thrown another slight query up for example if i run the program and use the following values it returns an average pace of 7.66 Per Minute when i would expect it to be 5.39 per minute. Perhaps i'm miss understanding the timespan function as i thought the output should be converted to minutes and seconds? Even if my equation is incorrect which i'm currently checking .66 should not be a value thats returned?

Values

Distance = 16
Time = 1hr 30mins 30secs

Neither of those values is correct. That time is 90.5 minutes and 90.5/16 is 5.65625, or 5.66. I suggest that you re-examine your logic and then do some debugging, i.e. set a breakpoint and then step through your code line by line, checking every value at each step.

 

[junkie_ball]

Sorry perhaps i didn't explain that very clearly of course in the example i showed the figure would be 5.66. What i was then trying to do was times the .66 by 60 so the time is displayed to the users as 5 Mins 39 Seconds not 5.66 Minutes. Excuse my ignorance on this subject but i have never used the Timespan function before. Is there a property of the timespan function i can use to achieve this or do i have to do the calculation manually in code to achieve the correct readout to the user?

 

[jmcilhinney]

The TimeSpan has properties Days, Hours, Minutes, Seconds, TotalDays, TotalHours, TotalSeconds. To show the difference, if you have a TimeSpan that represents 5 minutes and 39 seconds then:

Minutes = 5
Seconds = 39
TotalMinutes = 5.66
TotalSeconds = 339

I'll let you consider how to use the appropriate properties for what you need.

 

[junkie_ball]

Thanks for that did experiment last night with the properties and came
across those properties. Will have a little tingle with it later. Ifind it refreshing ur pointing me in the right direction without actually telling me straght out as much perfer to try and figure stuff out in my own head as I understand it better.

Will report back on how I get on.

 

[junkie_ball]

Hi thanks for all the help given jmcilhinney i now have the calculator displaying the correct outputs. I'm sure there's a nicer way to code this but it's working for me. This is what i ended up with in the end.

   Private Function PaceCalc(ByVal Hours As Integer, ByVal Minutes As Integer, ByVal Seconds As Integer, ByVal Distance As Decimal) As String


        ' Function Calculates average pace from data supplied from textboxes on form

        Dim Time As New TimeSpan(Hours, Minutes, Seconds)
        Dim TotalMinutes As Double = Time.TotalMinutes
        Dim Pace As Decimal

        Pace = TotalMinutes / Distance
        Dim PacePer As TimeSpan = ConvertMinutesToTimeSpan(Pace)

        PaceCalc = PacePer.Minutes.ToString & " Mins " & PacePer.Seconds.ToString & " Secs Per " & cmbDistanceUnit.Text  'TotalMinutes / Distance

    End Function

    Private Function ConvertMinutesToTimeSpan(ByVal TotalMinutes As Double) As TimeSpan

        'Converts TotalMinutes To TimeSpan

        If TotalMinutes >= Int(TimeSpan.MinValue.TotalMinutes) And TotalMinutes <= Int(TimeSpan.MaxValue.TotalMinutes) Then
            Dim ts As TimeSpan = TimeSpan.FromMinutes(TotalMinutes)
            Return ts
        End If
    End Function