I've been doing experiments with VBA and been getting into it. I'm not the best but a lot of times I can figure things out. But this...this is just insane. This makes no logical sense and I'm coming here for help. I enjoy "number mazes" so I sat down to create a number maze in Excel using VBA. The first screenshot is what the maze looks like before my code fills it in.
The maze without being filled in
The way this works is as follows : the largest room size possible is 7 by 7, so the largest number to be generated at any time is 7. However, there are some squares that will have different effects, either making you go back between 1 to 7 squares, or rewarding you with a random number of points. So to facilitate this in the easiest way possible there is the second screenshot, which is multiples of 7 up to 4. The section labeled "do nothing" is exactly that, just move tiles. Randomizing the numbers while also providing equal opportunity for 2 increments of 7, thus doubling the chances that those tiles will appear over the punishment or reward tile. (Again see screenshot 2). I did the math, figured out the best formula for turning say 16 into it's base 7 multiple (that being 2) and applied this formula to my code which is another screenshot.
Fairly simple, or so I thought.
I wrote the code, worked hard to get it working, and then, this happens. I'm getting numbers that are higher than 7, and yet lesser than or equal to 16, which shouldn't be possible. And the numbers i get are a constant range which is weird. I'm completely stumped, this shouldn't be happening and I cannot identify a pattern to this. Below is my code. This is just the strangest thing I've ever seen and I'm completely stumped. I never see anything greater than 21, so I know the logic for that part is correct. I never see 8, 9, 10, 11 or 13. But The numbers that are constantly generated is 12, 14, 15, and 16. Those 4 numbers consistently which is weird. I've quadrupled checked my logic, the only one that could cause issues is between the range 14 to 18, but why 12? Why just those numbers? I'm so stumped right now.
The results of pressing the button
Private Sub CommandButton1_Click()
Dim RandVal As Integer
Dim Multiplier As Integer
Dim rng As Range
For Each rng In Range("A1:AY51")
RandVal = ((28 * Rnd) 1)
If rng.Value = "" Then
If RandVal = 8 Or RandVal = 17 Or RandVal = 28 Or RandVal = 19 Or RandVal = 23 Or RandVal = 6 Or RandVal = 25 Or RandVal = 1 Or RandVal = 18 Or RandVal = 3 Or RandVal = 7 Or RandVal = 20 Or RandVal = 16 Or RandVal = 12 Then
If RandVal > 21 Then
RandVal = RandVal - (7 * 3)
rng.Value = rng.Value RandVal
ElseIf RandVal > 14 And RandVal <= 21 Then
RandVal = RandVal - (7 * 2)
rng.Value = rng.Value RandVal
ElseIf RandVal > 7 And RandVal <= 14 Then
RandVal = RandVal - 7
rng.Value = rng.Value RandVal
End If
rng.Value = rng.Value RandVal
End If
If RandVal = 10 Or RandVal = 13 Or RandVal = 5 Or RandVal = 14 Or RandVal = 22 Or RandVal = 4 Or RandVal = 9 Then
If RandVal > 21 Then
RandVal = RandVal - (7 * 3)
rng.Value = rng.Value RandVal
ElseIf RandVal > 14 And RandVal <= 21 Then
RandVal = RandVal - (7 * 2)
rng.Value = rng.Value RandVal
ElseIf RandVal > 7 And RandVal <= 14 Then
RandVal = RandVal - 7
rng.Value = rng.Value RandVal
End If
rng.Value = rng.Value RandVal
End If
If RandVal = 15 Or RandVal = 11 Or RandVal = 24 Or RandVal = 2 Or RandVal = 27 Or RandVal = 21 Or RandVal = 26 Then
If RandVal > 21 Then
RandVal = RandVal - (7 * 3)
rng.Value = rng.Value RandVal
ElseIf RandVal > 14 And RandVal <= 21 Then
RandVal = RandVal - (7 * 2)
rng.Value = rng.Value RandVal
ElseIf RandVal > 7 And RandVal <= 14 Then
RandVal = RandVal - 7
rng.Value = rng.Value RandVal
End If
End If
End If
Next rng
End Sub
CodePudding user response：
Several Points to address
1 Don't put code in a form control that isn't related to determining the state of the control and passing that state back.
Your 'CommandButton1_Click' thus becomes
Private Sub CommandButton1_Click()
RefreshBoard
End Sub
2.You have some If's that test multiple constants to select which action to take.
Such a structure , in your case, is better represented by a Select Case Structure
3.You have repetitive code for each of the multi if statements. This is better abstracted into its own Method. Thus your refresh board Method can be collapsed as follows
Select Case RandVal
Case 1, 3, 6, 7, 8, 12, 17, 18, 19, 20, 23, 25, 28
rng.Value = RefreshCell(rng.Value)
rng.Value = rng.Value RandVal
Case 4, 5, 9, 10, 13, 14, 22
rng.Value = RefreshCell(rng.Value)
rng.Value = rng.Value RandVal
Case 2, 11, 15, 21, 24, 26, 27
rng.Value = RefreshCell(rng.Value)
End Select
'and
Public Function RefreshCell(ByVal ipCellValue As Variant) As Variant
If RandVal > 21 Then
RandVal = RandVal - (7 * 3)
ipCellValue = ipCellValue RandVal
ElseIf RandVal > 14 Then
RandVal = RandVal - (7 * 2)
ipCellValue = ipCellValue RandVal
ElseIf RandVal > 7 Then
RandVal = RandVal - 7
ipCellValue = ipCellValue RandVal
End If
RefreshCell = ipCellValue
End Function
The second set of If Tests, now encapsulated in the RefreshCell Method contain unecessary tests. Eg. 'If >21' means than in the following ElseIf there is no need to do '<=21' because all numbers >21 have aleady been eliminated. You can only get to the else if clause if the number is <=21.
SO the whole code is
Private Sub CommandButton1_Click()
RefreshBoard
End Sub
Public Sub RefreshBoard()
Dim RandVal As Integer
Dim Multiplier As Integer ' Not used
Dim rng As Range
For Each rng In Range("A1:AY51")
RandVal = ((28 * Rnd) 1)
If rng.Value = "" Then
Select Case RandVal
Case 1, 3, 6, 7, 8, 12, 17, 18, 19, 20, 23, 25, 28
rng.Value = RefreshCell(rng.Value)
rng.Value = rng.Value RandVal
Case 4, 5, 9, 10, 13, 14, 22
rng.Value = RefreshCell(rng.Value)
rng.Value = rng.Value RandVal
Case 2, 11, 15, 21, 24, 26, 27
rng.Value = RefreshCell(rng.Value)
End Select
Next rng
End Sub
Public Function RefreshCell(ByVal ipCellValue As Variant) As Variant
If RandVal > 21 Then
RandVal = RandVal - (7 * 3)
ipCellValue = ipCellValue RandVal
ElseIf RandVal > 14 Then
RandVal = RandVal - (7 * 2)
ipCellValue = ipCellValue RandVal
ElseIf RandVal > 7 Then
RandVal = RandVal - 7
ipCellValue = ipCellValue RandVal
End If
RefreshCell = ipCellValue
End Function
- You are also constantly reading and writing individual cell. It would be more efficient to copy the excel range to a VBA array and work with the VBA array.
You would copy the updated array back to excel at the end of the refresh board method.
This use of an array requires implementing a nested for to iterate the board cell because you cannot write back to a value generated by a for each unless that value is an object and you are changing the contents of an object.
When you examine the revised code it becomes clearer, as @timwilliams has identified, that you might have some extraneous lines of code.
rng.Value = rng.Value RandVal
Edited to add this final point. I haven't read the rules so it may be that the fact it was possible to generate the RefreshCell method may reflect that you haven't correctly implemented the rules as you seem to be applying the same code to every value between 1 and 28.
CodePudding user response：
Unless I'm mis-reading something it seems like your code reduces to:
Private Sub CommandButton1_Click()
Dim rng As Range
For Each rng In Range("A1:AY51")
If rng.Value = "" Then
rng.Value = Application.RandBetween(1, 7)
End If
Next rng
End Sub
It's not clear why you have the different cases split up like that, but with the same logic applied to each block?