HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Autocompleting locations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
locationsautocompletingstackoverflow

Problem

Can this function be cleaner? It looks to me that I have too much nesting (Using + If + While).

Public Shared Function GetLocationsAutocomplete(ByRef userData As IUser, ByVal prefix As String) As List(Of AutocompleteItem)
Dim conn As DbConnection = DbProvider.GetDbConnection(ConnectionStringHelper.GetConnectionString(userData))
Dim retValue As List(Of AutocompleteItem) = Nothing

Const SQL As String = " SELECT DISTINCT TOP 20 Location " & _
" FROM userdetailstable WHERE ClientID=@ClientID " & _
" AND Location IS NOT NULL AND LEN(Location) > 0 AND Location LIKE @Filter "
Try
conn.Open()
Logger.WriteConnectionInfo("UserDataAccess.GetLocationsAutocomplete", Logger.ConnectionActionType.Opened)

' Getting data from DB:
Using cmd As DbCommand = DbProvider.GetDbCommand(SQL, conn)
cmd.Parameters.Add("@ClientID", DbType.Int32).Value = userData.ClientID
cmd.Parameters.Add("@Filter", DbType.String).Value = String.Format("%{0}%", prefix)
Using reader As DbDataReader = cmd.ExecuteReader()
If reader.HasRows Then
retValue = New List(Of AutocompleteItem)
' Populate retValue list with project names:
Dim item As AutocompleteItem = Nothing
While reader.Read()
item = New AutocompleteItem()
item.Value = reader.GetString("Location")
item.Label = item.Value
retValue.Add(item)
End While
End If
End Using
End Using
Finally
conn.Close()
conn.Dispose()
Logger.WriteConnectionInfo("SearchDataAccess.GetLocationsAutocomplete", Logger.ConnectionActionType.Closed)
End Try

Return retValue

End Function

Solution

Your comments should actually give you a hint.

' Getting data from DB:
' ...
' Populate retValue list with project names:


These sound like the beginnings of good function names to me.

Private Function GetAutocompleteItems(reader as DbDataReader) as List(Of AutocompleteItem)
If reader.HasRows Then
retValue = New List(Of AutocompleteItem)
' Populate retValue list with project names:
Dim item As AutocompleteItem = Nothing
While reader.Read()
item = New AutocompleteItem()
item.Value = reader.GetString("Location")
item.Label = item.Value
retValue.Add(item)
End While
End If

Return retValue
End Function


Which would then be called like this.

' Getting data from DB:
Using cmd As DbCommand = DbProvider.GetDbCommand(SQL, conn)
cmd.Parameters.Add("@ClientID", DbType.Int32).Value = userData.ClientID
cmd.Parameters.Add("@Filter", DbType.String).Value = String.Format("%{0}%", prefix)
Using reader As DbDataReader = cmd.ExecuteReader()
retValue = GetAutocompleteItems(reader)
End Using
End Using


As you can see, just extracting this one function out considerably reduced the nesting.

I don't see much else to pick on other than this.

Finally
        conn.Close()
        conn.Dispose()
        Logger.WriteConnectionInfo("SearchDataAccess.GetLocationsAutocomplete", Logger.ConnectionActionType.Closed)
    End Try


DbConnection obviously implements IDisposable, so why not use another Using block?

Code Snippets

' Getting data from DB:
' ...
' Populate retValue list with project names:
Finally
        conn.Close()
        conn.Dispose()
        Logger.WriteConnectionInfo("SearchDataAccess.GetLocationsAutocomplete", Logger.ConnectionActionType.Closed)
    End Try

Context

StackExchange Code Review Q#73707, answer score: 8

Revisions (0)

No revisions yet.