10
\$\begingroup\$

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
\$\endgroup\$
8
\$\begingroup\$

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?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.