WordPress 隨機文明 5 團隊選擇

問題描述

這個宏是為特定數量的 Civ V 玩家隨機抽取特定數量的文明選擇。它基於 43 種可能性的列表，while 循環中的 for 循環檢查已選擇的文明。恆定單元格引用指向工作表中使用 scroll-bars 獲得的值。使用基於變量的單元格引用來構造 two-column 設計中的結果。

有沒有什麼聰明的方式使我的代碼從美學和功能的角度更好？

Sub CIV_draw()
Range("K2:M50").ClearContents
Dim x As Integer
x = Cells(3, 3).Value
For i = 1 To x
    Cells(3 + (Cells(3, 7).Value + 2) * (i - 1), 11).Value = "Player " & i
    For j = 1 To Cells(3, 7).Value
        Dim Rand_CIV As String
        Dim RandNum As Integer
        Dim checkVal As Boolean
        checkVal = False
        While checkVal = False
            RandNum = CInt(Int(43 * Rnd())) + 1
            Rand_CIV = Cells(RandNum, 16).Value & " (" & Cells(RandNum, 15).Value & ")"
            For Z = 1 To 50
                If Cells(Z, 12) = Rand_CIV Then
                    Exit For
                End If
                If Z = 50 Then
                    checkVal = True
                End If
            Next Z
        Wend

        Cells((Cells(3, 7).Value + 2) * (i - 1) + (j + 3), 12).Value = Rand_CIV
    Next j
Next i
End Sub

我知道例如，我的重複檢查不是最佳的，我想獲得替代解決方案的建議。

最佳解決方法

那麼，Rubberduck 只表示一個問題。

Rubberduck Code Inspections – 3/8/2015 12:56:11 PM 1 issue found. Suggestion: Member ‘CIV_draw’ is implicitly Public – VBAProject.Module1, line 3

Implicit Public Member documentation

所以，你是一個好的開始。您可以通過顯式聲明子例程的範圍來糾正這一點。

Public Sub CIV_draw()

就我個人而言，我也會把這個前綴加在一起，只需調用這個例程 Draw()就可以了。下劃線在 VBA 中佔有特殊地位。它通常表示事件過程或接口實現，所以我將避免在其他地方的過程名稱中使用下劃線。

接下來我注意到，您正在調用 Range()和 Cells()方法，而不指定工作表。這意味着您在 ActiveSheet 上隱式調用這些方法。應該避免這種情況，因為在執行代碼時，用户將很少改變工作表。您應該創建一個工作表變量，該變量將在最初設置，然後使用它。

Dim ws As Worksheet
Set ws = ThisWorkbook.ActiveSheet

ws.Range("K2:M50").ClearContents
' ...

你的代碼中還有很多魔術數字，但是我會回頭看看。首先，我們來談談提取一些很有名的方法。隨機數邏輯是一個很好的解決方案。當讀這個方法時，我們並不在乎如何產生隨機數，只是我們得到一個。

RandNum = CInt(Int(43 * Rnd())) + 1

Private Function GetRandomNum() As Integer
    GetRandomNum = CInt(Int(43 * Rnd())) + 1
End Function

接下來是獲得隨機文明字符串的業務。

Private Function GetRandomCivilization(ByVal RandNum As Integer, ByVal ws As Worksheet) As String
    GetRandomCivilization = ws.Cells(RandNum, 16).Value & " (" & ws.Cells(RandNum, 15).Value & ")"
End Function

這已經使您的代碼更高級的可讀性。如果有人想要細節，他們可以挖掘這些私人功能。我們也通過這樣做完全消除了一個變量。

Dim RandCivilization As String
Dim checkVal As Boolean

checkVal = False
While checkVal = False
    RandCivilization = GetRandomCivilization(GetRandomNum(), ws)
    For Z = 1 To 50

接下來我們來清理你的 While 循環。

Dim checkVal As Boolean  checkVal = False While checkVal = False

接下來，讓我們翻轉邏輯來使用 Not 條件。

Dim checkVal As Boolean
checkVal = False

While Not checkVal

最後，給它一個更好的名字。

Dim endOfRange As Boolean
endOfRange = False

While Not endOfRange

下一個業務順序是將這個重複的邏輯提取到自己的方法中。

(ws.Cells(3, 7).Value + 2)

Private Function GetNumberOfCivilizations(ByVal ws As Worksheet)
    GetNumberOfCivilizations = ws.Cells(3, 7).Value
End Function

這使得代碼的當前狀態為此。

Public Sub Draw()
    Dim ws As Worksheet
    Set ws = ThisWorkbook.ActiveSheet

    ws.Range("K2:M50").ClearContents

    Dim x As Integer
    x = ws.Cells(3, 3).Value

    Dim numberOfCivs As Integer
    numberOfCivs = GetNumberOfCivilizations()

    Dim i As Long
    For i = 1 To x
        ws.Cells(3 + (numberOfCivs + 2) * (i - 1), 11).Value = "Player " & i

        Dim j As Long
        For j = 1 To numberOfCivs
            Dim RandCivilization As String
            Dim endOfRange As Boolean

            endOfRange = False
            While Not endOfRange
                RandCivilization = GetRandomCivilization(GetRandomNum(), ws)

                Dim z As Long
                For z = 1 To 50
                    If ws.Cells(z, 12) = RandCivilization Then
                        Exit For
                    End If
                    If z = 50 Then
                        endOfRange = True
                    End If
                Next z
            Wend

            ws.Cells((numberOfCivs + 2) * (i - 1) + (j + 3), 12).Value = RandCivilization
        Next j
    Next i
End Sub

Private Function GetNumberOfCivilizations(ByVal ws As Worksheet)
    GetNumberOfCivilizations = ws.Cells(3, 7).Value
End Function

Private Function GetRandomNum() As Integer
    GetRandomNum = CInt(Int(43 * Rnd())) + 1
End Function

Private Function GetRandomCivilization(ByVal RandNum As Integer, ByVal ws As Worksheet) As String
    GetRandomCivilization = ws.Cells(RandNum, 16).Value & " (" & ws.Cells(RandNum, 15).Value & ")"
End Function

這是一個改進，但我們仍然在處理很多數字，對我們來説並不意味着我們，直到我們回到實際的工作表。我們為它們定義一些常量值，以及一些負責從工作表獲取數據的函數。

Private Function GetNumberOfPlayers(ByVal ws As Worksheet) As Integer
    GetNumberOfPlayers = ws.Cells(3, 3).Value
End Function

還有另一個功能來獲取我們寫出結果的範圍。

Private Function GetResultsRange(ByVal ws As Worksheet) As Range
    Set GetResultsRange = ws.Range("K2:M50")
End Function

現在，請注意，我們正在引用結果範圍內的列位置，而不是相對於工作表的位置。一個人在只看代碼的時候更容易理解。

Public Sub Draw()
    Dim ws As Worksheet
    Set ws = ThisWorkbook.ActiveSheet

    Dim resultsRange As Range
    Set resultsRange = GetResultsRange(ws)
    resultsRange.ClearContents

    Dim numOfPlayers As Integer
    numberOfPlayers = GetNumberOfPlayers()

    Dim numberOfCivs As Integer
    numberOfCivs = GetNumberOfCivilizations()

    Dim i As Long
    For i = 1 To numberOfPlayers
        resultsRange.Cells(3 + (numberOfCivs + 2) * (i - 1), 1).Value = "Player " & i

        Dim j As Long
        For j = 1 To numberOfCivs
            Dim RandCivilization As String
            Dim endOfRange As Boolean

            endOfRange = False
            While Not endOfRange
                RandCivilization = GetRandomCivilization(GetRandomNum(), ws)

                Dim z As Long
                For z = 1 To resultsRange.Rows.Count
                    If resultsRange.Cells(z, 2) = RandCivilization Then
                        Exit For
                    End If
                    If z = resultsRange.Rows.Count Then
                        endOfRange = True
                    End If
                Next z
            Wend

            resultsRange.Cells((numberOfCivs + 2) * (i - 1) + (j + 3), 2).Value = RandCivilization
        Next j
    Next i
End Sub

在這一點上，你應該得到這個想法。繼續提取良好命名的函數，直到您的主程序中沒有重複或模糊的邏輯。這個代碼段將是我的下一個目標。

(numberOfCivs + 2) * (i - 1)

以及一些負責將數據寫入結果範圍的子站。

當我停止審查時，我的 IDE 中的代碼：

Option Explicit  Public Sub Draw()     Dim ws As Worksheet     Set ws = ThisWorkbook.ActiveSheet      Dim resultsRange As Range     Set resultsRange = GetResultsRange(ws)     resultsRange.ClearContents      Dim numOfPlayers As Integer     numberOfPlayers = GetNumberOfPlayers()      Dim numberOfCivs As Integer     numberOfCivs = GetNumberOfCivilizations()      Dim i As Long     For i = 1 To numberOfPlayers         resultsRange.Cells(3 + (numberOfCivs + 2) * (i - 1), 1).Value = "Player " & i          Dim j As Long         For j = 1 To numberOfCivs             Dim RandCivilization As String             Dim endOfRange As Boolean              endOfRange = False             While Not endOfRange                 RandCivilization = GetRandomCivilization(GetRandomNum(), ws)                  Dim z As Long                 For z = 1 To resultsRange.Rows.Count                     If resultsRange.Cells(z, 2) = RandCivilization Then                         Exit For                     End If                     If z = resultsRange.Rows.Count Then                         endOfRange = True                     End If                 Next z             Wend              resultsRange.Cells((numberOfCivs + 2) * (i - 1) + (j + 3), 2).Value = RandCivilization         Next j     Next i End Sub  Private Function GetNumberOfCivilizations(ByVal ws As Worksheet)     GetNumberOfCivilizations = ws.Cells(3, 7).Value End Function  Private Function GetRandomNum() As Integer     GetRandomNum = CInt(Int(43 * Rnd())) + 1 End Function  Private Function GetRandomCivilization(ByVal RandNum As Integer, ByVal ws As Worksheet) As String     GetRandomCivilization = ws.Cells(RandNum, 16).Value & " (" & ws.Cells(RandNum, 15).Value & ")" End Function  Private Function GetNumberOfPlayers(ByVal ws As Worksheet) As Integer     GetNumberOfPlayers = ws.Cells(3, 3).Value End Function  Private Function GetResultsRange(ByVal ws As Worksheet) As Range     Set GetResultsRange = ws.Range("K2:M50") End Function

次佳解決方法

列 $O:$P 是應用程序數據，它們不完全屬於您的 front-end 工作表。

我將這兩列複製到另一個工作表，插入標題行，添加標題 CivilizationName 和 LeaderName，然後將該範圍轉換為表。是什麼賦予了？桌子是最有用的 Excel 的強大功能，也可以使用它們！

在 VBA 方面的事情，它大大簡化了訪問數據 – 您可以給工作表一個編程名稱，所以我將工作表命名為表 Civilizations，並將表本身命名為 tblCivilizations(“tbl” 前綴有用於區分 Excel 公式中不同類型的命名範圍) 。

這意味着 VBA 現在可以訪問這個表：

Dim civilizationsTable As ListObject
Set civilizationsTable = Civilizations.ListObjects(1)

然後可以使用 civilizationsTable.ListRows 迭代行，這意味着您可以在 i 行的第 2 列中獲取值，如下所示：

civilizationsTable.ListRows(i).Range(ColumnIndex:=2)

這意味着這樣一條線：

Rand_CIV = Cells(RandNum, 16).Value & " (" & Cells(RandNum, 15).Value & ")"

可以這樣寫：

civName = civilizationsTable.ListRows(RandNum).Range(ColumnIndex:=1)
civLeader = civilizationsTable.ListRows(RandNum).Range(ColumnIndex:=2)
civCaption = civLeader & " (" & civName & ")"

更多的代碼，你會説？等等，我還沒完成

Public Enum CivilizationTableColumns
    CivilizationName = 1
    CivilizationLeader '= 2
End Enum

把它變成這樣：

civName = civilizationsTable.ListRows(RandNum).Range(ColumnIndex:=CivilizationName)
civLeader = civilizationsTable.ListRows(RandNum).Range(ColumnIndex:=CivilizationLeader)
civCaption = civLeader & " (" & civName & ")"

然後我們可以買到一個 ListRow 對象：

Set row = civilizationsTable.ListRows(RandNum)
civName = row.Range(ColumnIndex:=CivilizatioName)
civLeader = row.Range(ColumnIndex:=CivilizationLeader)
civCaption = civLeader & " (" & civName & ")"

魔數什麼魔數？ … 然後那個小塊可以被提取到自己的小功能中：

Private Function GetCivilizationCaption(ByVal index As Long)
    Set row = civilizationsTable.ListRows(index)
    civName = row.Range(ColumnIndex:=CivilizatioName)
    civLeader = row.Range(ColumnIndex:=CivilizationLeader)
    GetCivilizationCaption = civLeader & " (" & civName & ")"
End Function

這裏的外賣是因為缺乏抽象，很難看清楚發生了什麼。我們的小人類大腦喜歡抽象。證明 – 你寧願保持什麼，6 個月下來？

Cells((Cells(3, 7).Value + 2) * (i - 1) + (j + 3), 12).Value = Rand_CIV

要麼..

AssignCivilizationForPlayer player, civilizationCaption

參考文獻

Randomizing Civilization 5 team choice

注：本文內容整合自 Google/Baidu/Bing 輔助翻譯的英文資料結果。如果您對結果不滿意，可以加入我們改善翻譯效果：薇曉朵技術論壇。

WordPress 隨機文明 5 團隊選擇