Anders
Anders

Reputation: 12560

Suggestions to improve VB.NET URL shortener?

Here is my function (updated):

Public Shared Function shortenUrl(ByVal URL As String) As String
    Return shortenUrl(URL, 32)
End Function
Public Shared Function shortenUrl(ByVal URL As String, ByVal maxLength As Integer) As String
    If URL.Length > maxLength Then
        String.Format("{0}...{1}", URL.Substring(0, (maxLength / 2)), URL.Substring(URL.Length - ((maxLength / 2) - 3)))
    Else
        Return URL
    End If
End Function

I fixed the problem where it didn't return maxLength chars because it didn't take into account the ellipses.


It seems to me that it is too complicated; any suggestions, comments, concerns are more than welcome.

Upvotes: 1

Views: 1232

Answers (3)

Mark Brackett
Mark Brackett

Reputation: 85655

Well, I don't know if it's too complicated...but it is wrong. If I call shortenUrl(URL, 29) I'd expect the return would have a maximum length of 29 characters. Your code would give me 31. If I call it with a length of 30, I'll get back 33 characters. You're not including the inserted "...", and you're relying on rounding to get the substring lengths and dropping the remainder.

I'd add some param validation, and change it to:

Public Function shortenUrl2(ByVal URL As String, ByVal maxLength As Integer) As String
    Const middle as String = "..."
    If maxLength < 0 Then
       Throw New ArgumentOutOfRangeException("maxLength", "must be greater than or equal to 0")
    ElseIf String.IsNullOrEmpty(URL) OrElse URL.Length <= maxLength Then
       Return URL
    ElseIf maxLength < middle.Length Then
       Return URL.Substring(0, maxLength)
    End If

    Dim left as String = URL.Substring(0, CType(Math.Floor(maxLength / 2), Integer))
    Dim right as String = URL.Substring(URL.Length - (maxLength - left.Length - middle.Length))

    Return left & middle & right
End Function

Upvotes: 1

tom.dietrich
tom.dietrich

Reputation: 8347

Public Shared Function shortenUrl(ByVal URL As String, Optional ByVal maxLength As Integer = 29) As String
    If URL.Length > maxLength Then       
        Return String.Format("{0}...{1}", URL.Substring(0, maxLength / 2), URL.Substring(URL.Length - (maxLength / 2)))
    Else
        Return URL
    End If
End Function

Not sure why people hate optional arguments so much. They do the exact same thing and expose to the user what value will be defaulted if they don't provide one.

Upvotes: 0

Mitchel Sellers
Mitchel Sellers

Reputation: 63126

Why not do this?

Public Shared Function shortenUrl(ByVal URL As String) As String
    Return shortenUrl(URL, 29)
End Function
Public Shared Function shortenUrl(ByVal URL As String, ByVal maxLength As Integer) As String
    If URL.Length > maxLength Then
        Return String.Format("{0}...{1}", URL.Substring(0, maxLength / 2),URL.Substring(URL.Length - (maxLength / 2)))
    Else
        Return URL
    End If
End Function

That at least gets rid of all of the tempoary declarations

Upvotes: 1

Related Questions