1

I have reworked this macro for two days in a load of different ways to try to prevent steps from repeating but the range G2 step seems to run 3 or 4 times and the range G3 2 or 3 times. Does anyone have any ideas??

Private Sub Worksheet_Change(ByVal Target As Range)

    If Not Intersect(Target, Target.Worksheet.Range("G2")) Is Nothing Then
        Range("g4").Value = "Team"
        Range("g3").Value = "Division"
        Call check
        Exit Sub
    End If

    If Not Intersect(Target, Target.Worksheet.Range("G3")) Is Nothing Then
        Range("G4").Value = "Team"
        Call check
        Exit Sub
    End If

    If Not Intersect(Target, Target.Worksheet.Range("G4")) Is Nothing Then
        Call check
        Exit Sub
    End If

    If Not Intersect(Target, Target.Worksheet.Range("D4")) Is Nothing Then
        Call check
        Exit Sub
    End If
End Sub
Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
Belshazzar
  • 11
  • 1
  • 4
    you are changing the sheet accordingly so calling the sub, each time. you need to read through this http://www.ozgrid.com/forum/showthread.php?t=37042 on `enableevents` – Nathan_Sav Mar 20 '17 at 13:59

2 Answers2

3

Your Worksheet_Change has succumbed to three of the most common mistakes in an event driven worksheet/workbook sub procedure.

  • You are not disabling events while making modifications to the worksheet. Each change triggers another event and the Worksheet_Change tries to run on top of itself over and over until it crashes.
  • Target could be a single cell or many cells. You need to deal with the possibility of Target being many cells by using Intersect to only get the affected cells within your range of possibilities.
  • If you disable events for any reason, make sure to provide error control that turns them back on if everything goes south. Typically, this can be done just before exiting the Worksheet_Change but not if you are going to use Exit Sub.

Here is my version of your procedure.

Option Explicit

Private Sub Worksheet_Change(ByVal Target As Range)

    If Not Intersect(Target, Range("D4, G2:G4")) Is Nothing Then
        On Error GoTo Safe_Exit
        Application.EnableEvents = False
        Dim trgt As Range
        For Each trgt In Intersect(Target, Range("D4, G2:G4"))
            Select Case trgt.Address(0, 0)
                Case "G2"
                    Range("G3:G4") = Application.Transpose(Array("Division", "Team"))
                    'call check is below
                Case "G3"
                    Range("G4") = "Team"
                    'call check is below
                Case "D4", "G4"
                    'call check is below
            End Select

        Next trgt
        Call check
    End If

Safe_Exit:
    Application.EnableEvents = True
End Sub
2

Your code is in the Worksheet_Change event. Every time the worksheet is changed this event fires, including when your code changes it

Range("g4").Value = "Team"

Thus you're stuck in a potentially infinite loop. To avoid this disable events before making any changes

Private Sub Worksheet_Change(ByVal Target As Range)

    Application.EnableEvents = False ' this turns events off

    If Not Intersect(Target, Target.Worksheet.Range("G2")) Is Nothing Then
        Range("g4").Value = "Team"
        Range("g3").Value = "Division"
        Call check
        Application.EnableEvents = True
        Exit Sub
    End If

    If Not Intersect(Target, Target.Worksheet.Range("G3")) Is Nothing Then
        Range("G4").Value = "Team"
        Call check
        Application.EnableEvents = True
        Exit Sub
    End If

    If Not Intersect(Target, Target.Worksheet.Range("G4")) Is Nothing Then
        Call check
        Application.EnableEvents = True
        Exit Sub
    End If

    If Not Intersect(Target, Target.Worksheet.Range("D4")) Is Nothing Then
        Call check
        Application.EnableEvents = True
        Exit Sub
    End If

    Application.EnableEvents = True

End Sub

You might need to enable or disable events within the subs you're calling too.

BTW I'd check if you really need those Exit Subs, if not you can just disable events once at the start and re-enable again at the end.

Community
  • 1
  • 1
Absinthe
  • 3,258
  • 6
  • 31
  • 70
  • I somehow missed the lines re-enabling events. Could you edit so I can retract my down-vote? (it's currently locked in until an edit occurs) –  Mar 20 '17 at 15:22
  • Done. I assume the code worked for you? – Absinthe Mar 20 '17 at 21:19
  • Done. I assume the code works (it certainly looks correct) but I went a slightly different route with my own version. –  Mar 21 '17 at 00:21