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 团队选择