patternMinor
Autocompleting locations
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.
These sound like the beginnings of good function names to me.
Which would then be called like this.
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.
' 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 TryDbConnection 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 TryContext
StackExchange Code Review Q#73707, answer score: 8
Revisions (0)
No revisions yet.