vb.net - 如何重构一个类？ (VB.Net)

Question

我最近启动了一个项目，需要通过 DirectoryServices 与 LDAP 进行一些集成。我已经在其他应用程序中做到了这一点，所以我进入其中一个应用程序看看我是如何做到的——为什么要重新发明轮子呢？好吧，虽然这个轮子可以用，但它是几年前开发的，有点味道(它是木制的，牢固地固定在以前的车辆上，很难修理，并且可能会产生颠簸的行驶感觉)。

所以我心里想，现在是重构这只小狗的最佳时机，让它更便携、可重用、可靠、更容易配置等。现在一切都很好，但后来我开始感到有点不知所措开始。它应该是一个单独的图书馆吗？应该如何配置呢？应该使用 IoC 吗？ DI？

所以我的[诚然是主观的]问题是——给定一个相对较小但非常有用的类，如下所示，重构它的好方法是什么？您会问什么问题以及您如何决定实现或不实现什么？您如何看待配置灵 active 的界限？

[注意:请不要过多地攻击这段代码好吗？它是很久以前编写的，并且在内部应用程序中运行得非常好。]

Public Class AccessControl

Public Shared Function AuthenticateUser(ByVal id As String, ByVal password As String) As Boolean
    Dim path As String = GetUserPath(id)
    If path IsNot Nothing Then
        Dim username As String = path.Split("/")(3)
        Dim userRoot As DirectoryEntry = New DirectoryEntry(path, username, password, AuthenticationTypes.None)
        Try
            userRoot.RefreshCache()
            Return True
        Catch ex As Exception
            Return False
        End Try
    Else
        Return False
    End If
End Function

Private Shared Function GetUserPath(ByVal id As String) As String
    Dim root As DirectoryEntry = New DirectoryEntry("LDAP://XXXXX/O=YYYY", String.Empty, String.Empty, AuthenticationTypes.None)
    Dim searcher As New DirectorySearcher
    Dim results As SearchResultCollection
    Dim result As SearchResult
    Try
        With searcher
            .SearchRoot = root
            .PropertiesToLoad.Add("cn")
            .Filter = String.Format("cn={0}", id)
            results = .FindAll()
        End With
        If results.Count > 0 Then
            result = results(0)
            Return result.Path.ToString()
        Else
            Return Nothing
        End If
    Catch ex As Exception
        Return Nothing
    End Try
End Function

Public Shared Function GetUserInfo(ByVal id As String) As UserInfo
    Dim root As DirectoryEntry = New DirectoryEntry("LDAP://XXXXX/O=YYYY", String.Empty, String.Empty, AuthenticationTypes.None)
    Dim searcher As New DirectorySearcher
    Dim results As SearchResultCollection
    Dim props() As String = {"id", "sn", "mail", "givenname", "container", "cn"}
    Try
        With searcher
            .SearchRoot = root
            .PropertiesToLoad.AddRange(props)
            .Filter = String.Format("cn={0}", id)
            results = .FindAll()
        End With
        If results.Count > 0 Then
            Dim properties As PropertyCollection = results(0).GetDirectoryEntry().Properties
            Dim user As New UserInfo(properties("id").Value)
            user.EmailAddress = properties("mail").Item(0).ToString
            user.FirstName = properties("givenname").Item(0).ToString
            user.LastName = properties("sn").Item(0).ToString
            user.OfficeLocation = properties("container").Item(0).ToString
            Return user
        Else
            Return New UserInfo
        End If
    Catch ex As Exception
        Return Nothing
    End Try
End Function

Public Shared Function IsMemberOfGroup(ByVal id As String, ByVal group As String) As Boolean
    Dim root As DirectoryEntry = New DirectoryEntry("LDAP://XXXXX/O=YYYY", String.Empty, String.Empty, AuthenticationTypes.None)
    Dim searcher As New DirectorySearcher
    Dim results As SearchResultCollection
    Dim result As SearchResult
    Dim props() As String = {"cn", "member"}
    Try
        With searcher
            .SearchRoot = root
            .PropertiesToLoad.AddRange(props)
            .Filter = String.Format("cn={0}", group)
            results = .FindAll()
        End With
        If results.Count > 0 Then
            For Each result In results
                Dim members As PropertyValueCollection = result.GetDirectoryEntry().Properties("member")
                Dim member As String
                For i As Integer = 0 To members.Count - 1
                    member = members.Item(i).ToString
                    member = member.Substring(3, member.IndexOf(",") - 3).ToLowerInvariant
                    If member.Contains(id.ToLowerInvariant) Then Return True
                Next
            Next
        End If
        Return False
    Catch ex As Exception
        Return False
    End Try
End Function

Public Shared Function GetMembersOfGroup(ByVal group As String) As List(Of String)
    Dim groupMembers As New List(Of String)
    Dim root As DirectoryEntry = New DirectoryEntry("LDAP://XXXXX/O=YYYY", String.Empty, String.Empty, AuthenticationTypes.None)
    Dim searcher As New DirectorySearcher
    Dim results As SearchResultCollection
    Dim result As SearchResult
    Dim props() As String = {"cn", "member"}
    Try
        With searcher
            .SearchRoot = root
            .PropertiesToLoad.AddRange(props)
            .Filter = String.Format("cn={0}", group)
            results = .FindAll()
        End With
        If results.Count > 0 Then
            For Each result In results
                Dim members As PropertyValueCollection = result.GetDirectoryEntry().Properties("member")
                Dim member As String
                For i As Integer = 0 To members.Count - 1
                    member = members.Item(i).ToString
                    member = member.Substring(3, member.IndexOf(",") - 3).ToLowerInvariant
                    groupMembers.Add(member)
                Next
            Next
        End If
    Catch ex As Exception
        Return Nothing
    End Try
    Return groupMembers
End Function

End Class  

说明:
- 有一个单独的用户类(简单的 poco)
- 没有组类，因为现在使用的只是 ids 列表，不过添加可能会很有用

Answer 1

以下是重构代码示例的示例:

Public Class AccessControl

    Public Shared Function AuthenticateUser(ByVal id As String, ByVal password As String) As Boolean
        Dim path As String
        Dim username As String
        Dim userRoot As DirectoryEntry

        path = GetUserPath(id)

        If path.Length = 0 Then
            Return False
        End If

        username = path.Split("/")(3)
        userRoot = New DirectoryEntry(path, username, password, AuthenticationTypes.None)

        Try
            userRoot.RefreshCache()
            Return True
        Catch ex As Exception
            ' Catching Exception might be accepted way of determining user is not authenticated for this case
            ' TODO: Would be better to test for specific exception type to ensure this is the reason
            Return False
        End Try
    End Function

    Private Shared Function GetUserPath(ByVal id As String) As String
        Dim results As SearchResultCollection
        Dim propertiesToLoad As String()

        propertiesToLoad = New String() {"cn"}
        results = GetSearchResultsForCommonName(id, propertiesToLoad)

        If results.Count = 0 Then
            Return String.Empty
        Else
            Debug.Assert(results.Count = 1)
            Return results(0).Path
        End If
    End Function

    Public Shared Function GetUserInfo(ByVal id As String) As UserInfo
        Dim results As SearchResultCollection
        Dim propertiesToLoad As String()

        propertiesToLoad = New String() {"id", "sn", "mail", "givenname", "container", "cn"}
        results = GetSearchResultsForCommonName(id, propertiesToLoad)

        If results.Count = 0 Then
            Return New UserInfo
        End If

        Debug.Assert(results.Count = 1)
        Return CreateUser(results(0).GetDirectoryEntry().Properties)
    End Function

    Public Shared Function IsMemberOfGroup(ByVal id As String, ByVal group As String) As Boolean
        Dim allMembersOfGroup As List(Of String)
        allMembersOfGroup = GetMembersOfGroup(group)
        Return allMembersOfGroup.Contains(id.ToLowerInvariant)
    End Function

    Public Shared Function GetMembersOfGroup(ByVal group As String) As List(Of String)
        Dim results As SearchResultCollection
        Dim propertiesToLoad As String()

        propertiesToLoad = New String() {"cn", "member"}
        results = GetSearchResultsForCommonName(group, propertiesToLoad)

        Return ConvertMemberPropertiesToList(results)
    End Function

    Private Shared Function GetStringValueForPropertyName(ByVal properties As PropertyCollection, ByVal propertyName As String) As String
        Return properties(propertyName).Item(0).ToString
    End Function

    Private Shared Function CreateUser(ByVal userProperties As PropertyCollection) As UserInfo
        Dim user As New UserInfo(userProperties("id").Value)
        With user
            .EmailAddress = GetStringValueForPropertyName(userProperties, "mail")
            .FirstName = GetStringValueForPropertyName(userProperties, "givenname")
            .LastName = GetStringValueForPropertyName(userProperties, "sn")
            .OfficeLocation = GetStringValueForPropertyName(userProperties, "container")
        End With
        Return user
    End Function

    Private Shared Function GetValueFromMemberProperty(ByVal member As String) As String
        Return member.Substring(3, member.IndexOf(",") - 3).ToLowerInvariant
    End Function

    Private Shared Function ConvertMemberPropertiesToList(ByVal results As SearchResultCollection) As List(Of String)
        Dim result As SearchResult
        Dim memberProperties As PropertyValueCollection
        Dim groupMembers As List(Of String)

        groupMembers = New List(Of String)
        For Each result In results
            memberProperties = result.GetDirectoryEntry().Properties("member")
            For i As Integer = 0 To memberProperties.Count - 1
                groupMembers.Add(GetValueFromMemberProperty(memberProperties.Item(i).ToString))
            Next
        Next
        Return groupMembers
    End Function

    Private Shared Function GetSearchResultsForCommonName(ByVal commonName As String, ByVal propertiesToLoad() As String) As SearchResultCollection
        Dim results As SearchResultCollection
        Dim searcher As New DirectorySearcher
        With searcher
            .SearchRoot = GetDefaultSearchRoot()
            .PropertiesToLoad.AddRange(propertiesToLoad)
            .Filter = String.Format("cn={0}", commonName)
            results = .FindAll()
        End With
        Return results
    End Function

    Private Shared Function GetDefaultSearchRoot() As DirectoryEntry
        Return New DirectoryEntry("LDAP://XXXXX/O=YYYY", String.Empty, String.Empty, AuthenticationTypes.None)
    End Function

End Class

我认为你可以通过提取常量等来进一步解决这个问题，但这会消除大部分重复等。让我知道你的想法。

方式，太晚了，我意识到......但也想解决您提出的一些问题。请参阅http://chrismelinn.wordpress.com/2011/06/18/questions-before-refactoring-2/