Mdthbs

I have a function I have written in VB for use in SSRS 2012. It takes an integer and converts it to full text duration - for example 900 becomes "15 minutes", 9000 becomes "2 hours 30 minutes", and 7201 is "2 hours 1 second".

The code works, and is:

Public Function SecondsFullText(ByVal TotalSeconds As Integer) As String



    Dim hours As Integer = new Integer()

    Dim minutes As Integer = new Integer()

    Dim seconds As Integer = new Integer()



    Dim hourString as String = ""

    Dim minuteString as String = ""

    Dim secondString as String = ""



    hours = floor(TotalSeconds / 3600)

    minutes = floor((TotalSeconds mod 3600) / 60)

    seconds = (TotalSeconds mod 3600) mod 60



    If hours = 1 Then

        hourString = Cstr(hours) & " hour"

    Else If hours > 1 Then

        hourString = Cstr(hours) & " hours"

    End If



    If minutes = 1 Then

        minuteString = Cstr(minutes) & " minute"

    Else If minutes > 1 Then

        minuteString = Cstr(minutes) & " minutes"

    End If



    If seconds = 1 Then

        secondString = Cstr(seconds) & " second"

    Else If seconds > 1 Then

        secondString = Cstr(seconds) & " seconds"

    End If



    If hours > 0 and (minutes > 0 or seconds > 0) Then

        hourString = hourString & " "

    End If



    If minutes > 0 and seconds > 0 Then

        minuteString = minuteString & " "

    End If



    return hourString & minuteString & secondString

End Function

My question is: how do I make this better? It seems very clunky, with too many IFs to handle the different possibilities for single/plural, spaces, etc. I feel that this can be better, but I'm not sure how.

This is called in Report Builder 3.0 / SSRS 2012 with this expression: =Code.SecondsFullText(Parameters!Integer.Value)

You should look into the TimeSpan struct, which provides a nice method TimeSpan.FromSeconds().

Having filled a TimeSpan struct by e.g calling the FromSeconds() method, you can just access its properties Seconds, Minutes and Hours.

Instead of concating strings like you do, you should consider to use a StringBuilder.

To get rid of the If ... Else If you can just use the singular and only if the value is greater than 1 you add a s to the string/StringBuilder.

Based on the .NET Naming Guidelines method parameters should be named using camelCase casing hence TotalSeconds should be totalSeconds

Implementing the mentioned points will look like so

Public Function SecondsFullText(ByVal totalSeconds As Double) As String



    Dim timeSpan As TimeSpan = TimeSpan.FromSeconds(totalSeconds)

    Dim stringBuilder As StringBuilder = New StringBuilder()



    Dim hours As Integer = timeSpan.Hours

    Dim minutes As Integer = timeSpan.Minutes

    Dim seconds As Integer = timeSpan.Seconds



    stringBuilder.Append(ToText(hours, "hour"))

    If hours > 0 AndAlso (minutes > 0 OrElse seconds > 0) Then

        stringBuilder.Append(" ")

    End If

    stringBuilder.Append(ToText(minutes, "minute"))

    If minutes > 0 AndAlso seconds > 0 Then

        stringBuilder.Append(" ")

    End If

    stringBuilder.Append(ToText(seconds, "second"))

    Return stringBuilder.ToString()



End Function



Private Function ToText(value As Integer, singularName As String) As String



    If value = 0 Then Return String.Empty

    If value = 1 Then Return String.Format("{0} {1}", value, singularName)

    Return String.Format("{0} {1}s", value, singularName)



End Function

As @Hesclacher has given a fantastic answer, which is not only one that I have upvoted but I would personally mark it as the correct answer if I had the power, I am hesitant to improve upon it. But I would like to offer constructive feedback on your original code.

I have a personal distinction between being a VB Coder and a .NET Developer. I think you can use VB.NET and be what I define to be a .NET Developer - that is you are making traditional .NET calls like any C# dev would use. I define a VB Coder has someone who still thinks and codes with VBA in mind, and such a person relies upon some of the compatibility calls, such as CStr.

According to my own such definitions, a VB Coder would use CStr whereas a .NET Developer would use String.Format (as did Heschalcher) or better yet, Interpolated Strings.

VB Coder:

hourString = Cstr(hours) & " hours"

.NET Developer:

hourString = String.Format("{0} hours", hours)

Or

hourString = $"{hours} hours"

To calculate the number of hours, if you aren't going to use something nice like TimeSpan, I would change this line of code:

hours = floor(TotalSeconds / 3600)

To simply use integer division:

hours = TotalSeconds  3600

Also, VB.NET does support short-circuited conditionals (unlike VBA), so this line of code:

If hours > 0 and (minutes > 0 or seconds > 0) Then

could be changed to:

If hours > 0 AndAlso (minutes > 0 OrElse seconds > 0) Then

This example employs short-circuiting with the AndAlso and OrElse operators.

Again, Heschachler's answer with TimeSpan and StringBuilder is spot-on.

Building on @RickDavin.

VB.Net also has a robust If() function that shortcuts the checks (unlike VBA's IIf() function)

If hours = 1 Then

    hourString = Cstr(hours) & " hour"

Else If hours > 1 Then

    hourString = Cstr(hours) & " hours"

End If

Becomes

hourstring = If(hours = 1, "hour", "hours")

@Heslacher's function can then be:

Private Function ToText(value As Integer, singularName As String) As String

    Return If(value = 0, String.Empty, String.Format("{0} {1}{2}", value, singularName, If(value=1,"","s")))

End Function

Note, I am still in the VB coder camp, and my kludge to the string builder format above is based some limited practice.