procedure too large vba for excel

Question

I am not used to writing code. I normally generate my code via macro and I am facing this issue. Can someone please help me?

Sub Test()

    Dim WorkRng As Range
    Dim Rng As Range
    Dim xOffsetColumn As Integer

    Set WorkRng = Intersect(Application.ActiveSheet.Range("B8:B38"), Target)
    xOffsetColumn = 19

    If Not WorkRng Is Nothing Then
        Application.EnableEvents = False

        For Each Rng In WorkRng
            If Not VBA.IsEmpty(Rng.Value) Then
                Rng.Offset(0, xOffsetColumn).Value = Now
                Rng.Offset(0, xOffsetColumn).NumberFormat = "mm/dd/yyyy, hh:mm:ss"
            Else
                Rng.Offset(0, xOffsetColumn).ClearContents
            End If
        Next

        Application.EnableEvents = True
    End If

    Dim WorkRng1 As Range
    Dim Rng1 As Range
    Dim xOffsetColumn1 As Integer

    Set WorkRng1 = Intersect(Application.ActiveSheet.Range("C8:C38"), Target)
    xOffsetColumn1 = 18

    If Not WorkRng1 Is Nothing Then

        For Each Rng1 In WorkRng1
            If Not VBA.IsEmpty(Rng1.Value) Then
                Rng1.Offset(0, xOffsetColumn1).Value = Now
                Rng1.Offset(0, xOffsetColumn1).NumberFormat = "mm/dd/yyyy, hh:mm:ss"
            Else
                Rng1.Offset(0, xOffsetColumn1).ClearContents
            End If
        Next

        Application.EnableEvents = True
    End If

    ....................................
    ..............................

    Dim WorkRng132 As Range
    Dim Rng132 As Range
    Dim xOffsetColumn132 As Integer

    Set WorkRng132 = Intersect(Application.ActiveSheet.Range("EJ8:EJ38"), Target)
    xOffsetColumn132 = 1

    If Not WorkRng132 Is Nothing Then

        For Each Rng132 In WorkRng132
            If Not VBA.IsEmpty(Rng132.Value) Then
                Rng132.Offset(0, xOffsetColumn132).Value = Now
                Rng132.Offset(0, xOffsetColumn132).NumberFormat = "mm/dd/yyyy, hh:mm:ss"
            Else
                Rng132.Offset(0, xOffsetColumn132).ClearContents
            End If
        Next

        Application.EnableEvents = True
    End If

End Sub

I can already tell that you could easily, in one loop, cycle through all those ranges (instead of having 132 blocks of the same code). — dwirony
– dwirony, Commented Aug 20, 2018 at 16:03
Voted to re-open. The answers suggested in the linked question are a poor response to the OP's problem - splitting a verbose procedure into many sub-procedures just pushed the problem around a little but, and does nothing to address the root cause of the issue here. It would be more fruitful to point the OP toward the concept of refactoring and parameterised sub-procedures. — Tim Williams
– Tim Williams, Commented Aug 20, 2018 at 16:06
Despite the link to the duplicate question.. do not split this up. Anytime you find yourself repeating code, write a function. The only thing that is changing between each iteration appears to be the range. A loop with this function would solve your issue. — JosephC
– JosephC, Commented Aug 20, 2018 at 16:06
You can give yourself an example based on the other code, thansk — Hoang The Tai
– Hoang The Tai, Commented Aug 20, 2018 at 16:10
Can you help me understand how xOffsetColumn1 works? Is it just 1 for 100+ loops? — Marcucciboy2
– Marcucciboy2, Commented Aug 20, 2018 at 16:12

Tim Williams · Accepted Answer · 2018-08-20 23:54:09Z

7

One useful maxim in programming is Don't Repeat Yourself (DRY) - duplicated code is longer, harder to understand, and difficult to maintain.

There's a clear repeating pattern in your code. This block:

Dim WorkRng As Range
Dim Rng As Range
Dim xOffsetColumn As Integer

Set WorkRng = Intersect(Application.ActiveSheet.Range("B8:B38"), Target)
xOffsetColumn = 19

If Not WorkRng Is Nothing Then
    Application.EnableEvents = False

    For Each Rng In WorkRng
        If Not VBA.IsEmpty(Rng.Value) Then
            Rng.Offset(0, xOffsetColumn).Value = Now
            Rng.Offset(0, xOffsetColumn).NumberFormat = "mm/dd/yyyy, hh:mm:ss"
        Else
            Rng.Offset(0, xOffsetColumn).ClearContents
        End If
    Next

    Application.EnableEvents = True
End If

Can be refactored into a re-usable method with two parameters:

Sub Test()
    '....
    ProcessRange Application.Intersect(Me.Range("B8:B38"), Target), 19
    ProcessRange Application.Intersect(Me.Range("C8:C38"), Target), 18
    'etc for the other ranges
    '....
End sub


'subprocedure
Sub ProcessRange(WorkRng As Range, offsetCol as Long)
    Dim Rng As Range
    If Not WorkRng Is Nothing Then
        Application.EnableEvents = False
        For Each Rng In WorkRng
            With Rng.Offset(0, offsetCol)
            If Not VBA.IsEmpty(Rng.Value) Then
                .Value = Now
                .NumberFormat = "mm/dd/yyyy, hh:mm:ss"
            Else
                .ClearContents
            End If
            End With
        Next
        Application.EnableEvents = True
    End If

End Sub

edited Aug 20, 2018 at 23:54

answered Aug 20, 2018 at 16:12

Tim Williams

169k8 gold badges104 silver badges143 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

dwirony Over a year ago

Might I suggest a loop of say, 2 to 140 (B to EJ), where a UDF can get the letter value of the number, that way he doesn't need 140 lines of ProcessRange? Although I'm not sure how he's getting that column offset number...

Siddharth Rout Over a year ago

Nicely done.. I almost closed this post as a duplicate... :D

Hoang The Tai Over a year ago

I do not understand this paragraph'.... ProcessRange Application.Intersect(Me.Range("B8:B38"), Target), 19 ProcessRange Application.Intersect(Me.Range("C8:C38"), Target), 18 '.... 'subprocedure

Tim Williams Over a year ago

Those two lines are in your main procedure, and call ProcessRange passing two parameters, a range and a column number.

Tim Williams Over a year ago

@dwirony - I didn't spend a long time looking at the next level up: likely what you suggest would be the next step in refactoring, but without the full procedure it's difficult to be certain exactly what's going on.

Collectives™ on Stack Overflow

procedure too large vba for excel

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related