0

Having trouble executing both Worksheet_Change events correctly. Image below show my results, when modifying column B, column M does nothing. When modifying column L, column N updates as expected but only on row 2. Every other subsequent change to B or M results in N:2 updating to the current time again. My desired outcome is that when Col B is updated I record a time stamp in Col M and the same when Col L updates that I get a time stamp in Col N. Example of Excel Error

My current code is here:

Option Explicit

Private Sub Worksheet_Change(ByVal Target As Range)
    Application.EnableEvents = False
    
    Dim rng As Range
    Dim rng2 As Range
    
    If Not Intersect(Target, Columns("B"), Target.Parent.UsedRange) Is Nothing Then
        On Error GoTo Safe_Exit
        For Each rng In Intersect(Target, Columns("B"), Target.Parent.UsedRange)
            If CBool(Len(rng.Value2)) And Not CBool(Len(rng.Offset(0, 11).Value2)) Then
            rng.Offset(0, 11) = Now
            ElseIf Not CBool(Len(rng.Value2)) And CBool(Len(rng.Offset(0, 11).Value2)) Then
            rng.Offset(0, 11) = vbNullString
            End If
        Next rng
        Application.EnableEvents = True
    End If
    

    ElseIf Not Intersect(Target, Columns("L"), Target.Parent.UsedRange) Is Nothing Then
        On Error GoTo Safe_Exit
        For Each rng2 In Intersect(Target, Columns("L"), Target.Parent.UsedRange)
            If CBool(Len(rng2.Value2)) And Not CBool(Len(rng2.Offset(0, 2).Value2)) Then
                rng2.Offset(0, 2) = Now
            ElseIf Not CBool(Len(rng2.Value2)) And CBool(Len(rng2.Offset(0, 2).Value2)) Then
                rng2.Offset(0, 2) = vbNullString
            End If
        Next rng2
        Application.EnableEvents = True
    End If
Safe_Exit:
End Sub
Siddharth Rout
  • 147,039
  • 17
  • 206
  • 250
  • 1
    Side note, but you need `Application.EnableEvents = True` to be *after* `Safe_Exit:`, otherwise if an error occurs you'll never enable events. – BigBen Oct 05 '22 at 13:20
  • 1
    Agreed with @BigBen on the events part. I have mentioned about it [HERE](https://stackoverflow.com/questions/13860894/why-ms-excel-crashes-and-closes-during-worksheet-change-sub-procedure/13861640#13861640) – Siddharth Rout Oct 05 '22 at 13:26
  • Also the `End If` before the first `ElseIf` doesn't make sense. Indented the code to show what i am referrring to – Siddharth Rout Oct 05 '22 at 13:34
  • `CBool(Len(.Value))` is a strange alternative to `Not IsEmpty(.Value)`. It has the unfortunate side-effect of erroring out when it encounters worksheet errors like `#DIV/0!` or `#NAME?` where `Not IsEmpty` still returns `True`. Perhaps an error on your sheet is interrupting the normal behavior of this code? – Toddleson Oct 05 '22 at 13:35
  • Seems like this whole thing could be simplified... you've basically designating a column to work with based on where a change is, so you only need the "do" part once. Will try and mark up: – Cyril Oct 05 '22 at 13:55

2 Answers2

0

Mock-up, untested, change of code to simplify as you're doing the same actions in two spots:

Option Explicit

Private Sub Worksheet_Change(ByVal Target As Range)
    Application.EnableEvents = False
    Dim columnLetter as String
    Select Case Target.Column
        Case 2 'B
            columnLetter = "M"
        Case 12 'L
            columnLetter = "N"
        Case Else
            Goto Safe_Exit
    End Select
    Dim loopRng as Range
    For Each loopRng In Range(Cells(Target.Row, Target.Column),Cells(Target.End(xlDown).Row,Target.Column)
        If IsEmpty(loopRng) = True And IsEmpty(Cells(loopRng.Row,columnLetter)) = False Then
            Cells(loopRng.Row,columnLetter) = Now
        ElseIf IsEmpty(loopRng) = False And IsEmpty(Cells(loopRng.Row,columnLetter)) = True Then
            Cells(loopRng.Row,columnLetter) = vbNullString
        End If
    Next loopRng
    'Columns(columnLetter).NumberFormat = "yyyy/mm/dd"
    Application.EnableEvents = True
Safe_Exit:
Application.EnableEvents = True
End Sub

Note that the IsEmpty() = True is important... when using an If case, you need to specify for each condition, otherwise the implicit detection will fail.


Edit1: Removed Intersect from loop, whereas the range i've listed will need corrected... it at least references a specific range, now.

Edit2: Removing .Offset and working with specific column references in cells().

Cyril
  • 6,448
  • 1
  • 18
  • 31
  • At first rng was not defined, changed to 'looprng' and it produced the same results I was previously getting. – Doug Sleigh Oct 05 '22 at 14:20
  • @DougSleigh Just stepping back; looks like you're using an intersect for a range and that's essentially going to be an issue with the `for each` loop. Basically it hits one item, the actual target, and that's the end of it. Needs to be more direct/dynamic to get the looped range appropriate – Cyril Oct 05 '22 at 14:39
  • I have been trying to update with a more dynamic range and failing to get the desired result. With the most recent updates, I am still only every getting the same result as before. – Doug Sleigh Oct 06 '22 at 18:52
  • @DougSleigh Then let's remove the offsets and start working with columns by name... let me do an update. – Cyril Oct 07 '22 at 13:54
  • This code stopped working completely, no data was populated in the target cells after this. – Doug Sleigh Oct 07 '22 at 15:19
0

I tried this version of my original code and it started to work for some reason.

 Option Explicit

    Private Sub Worksheet_Change(ByVal Target As Range)
If Not Intersect(Target, Columns("B"), Target.Parent.UsedRange) Is Nothing Then
On Error GoTo Safe_Exit
Application.EnableEvents = False
Dim rng As Range
For Each rng In Intersect(Target, Columns("B"), Target.Parent.UsedRange)
If CBool(Len(rng.Value2)) And Not CBool(Len(rng.Offset(0, 11).Value2)) Then
rng.Offset(0, 11) = Now
ElseIf Not CBool(Len(rng.Value2)) And CBool(Len(rng.Offset(0, 11).Value2)) Then
rng.Offset(0, 11) = vbNullString
End If
Next rng
End If
If Not Intersect(Target, Columns("L"), Target.Parent.UsedRange) Is Nothing Then
On Error GoTo Safe_Exit
Application.EnableEvents = False
For Each rng In Intersect(Target, Columns("L"), Target.Parent.UsedRange)
If CBool(Len(rng.Value2)) And Not CBool(Len(rng.Offset(0, 2).Value2)) Then
rng.Offset(0, 2) = Now
ElseIf Not CBool(Len(rng.Value2)) And CBool(Len(rng.Offset(0, 2).Value2)) Then
rng.Offset(0, 2) = vbNullString
End If
Next rng
End If
Safe_Exit:
Application.EnableEvents = True
End Sub