If statement not properly applying to criteria

Question

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.

The rules for how spaces are calculated and what to do on those tiles. I'll add color formatting to the tiles later

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

user_7177346 · Answer

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.

user_478884 · Answer

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?

If statement not properly applying to criteria

2 Answers