Results 1 to 4 of 4
  1. #1
    New Lounger
    Join Date
    May 2009
    Posts
    2
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Hi all,

    My first post!
    I am attempting to use the VBA Macro below to clear cell contents based on a value being set in Cell range L9:L67.
    The Macro was running normally until I added the last two If statements and it now appears to remain in a continual loop.

    I am new to writing VBA scripts so I would guess I am committing some massive syntax error here, or at the very least the structure of my macro is very inefficient.

    Can I ask for suggestions on the code, and some opinions as to how to improve its efficiency or as a minimum stop it from looping?

    Many Thanks,

    -Steve

    Private Sub Worksheet_Calculate()
    Dim cl As Range
    For Each cl In Range("L9:L67")
    If cl.Value = "ValueA" And UCase(Range("AB9")) = "ALLOW RUN" Then cl.Offset(, 3).ClearContents
    If cl.Value = "ValueA" And UCase(Range("AB9")) = "ALLOW RUN" Then Range("AB9").ClearContents
    If cl.Value = "ValueB" And Range("T8") = 1 And UCase(Range("AC9")) <> "1st Set" Then cl.Offset(, 9) = "1st"
    If cl.Value = "ValueB" And Range("T8") = 1 And UCase(Range("AC9")) <> "1st Set" Then Range("AC9") = "1st Set"
    Next cl
    End Sub

    Here is the same code but with comments explaining my reasoning...

    Private Sub Worksheet_Calculate()
    Dim cl As Range

    For Each cl In Range("L9:L67")
    ! If first run (AB9="ALLOW RUN") and ValueA has been detected, then
    ! clear cell in same row of column O.
    If cl.Value = "ValueA" And UCase(Range("AB9")) = "ALLOW RUN" Then cl.Offset(, 3).ClearContents

    ! If first run (AB9="ALLOW RUN") and ValueA has been detected, then
    ! clear cell AB9, which will prevent cell in Column O from being continually
    ! cleared by first IF statement
    If cl.Value = "ValueA" And UCase(Range("AB9")) = "ALLOW RUN" Then Range("AB9").ClearContents

    ! If ValueB has been detected AND T8 equals 1 AND AC9 is not equal to
    ! "1st Set" (meaning this is first run), then set the same row identifer but
    ! in row U to "1st"
    If cl.Value = "ValueB" And Range("T8") = 1 And UCase(Range("AC9")) <> "1st Set" Then cl.Offset(, 9) = "1st"

    ! If ValueB has been detected AND T8 equals 1 AND AC9 is not equal to
    ! "1st Set" (meaning this is first run), then set AC9 to "1st Set",
    ! preventing row U from being cleared by the previous If statement.
    If cl.Value = "ValueB" And Range("T8") = 1 And UCase(Range("AC9")) <> "1st Set" Then Range("AC9") = "1st Set"

    Next cl
    End Sub

  2. #2
    Platinum Lounger
    Join Date
    Feb 2001
    Location
    Weert, Limburg, Netherlands
    Posts
    4,812
    Thanks
    0
    Thanked 0 Times in 0 Posts
    I strongly suspect that the two additional statements are causing Excel to recalculate, which in turn fires the calculate event that caused that calculation.
    You need to prevent this so-called event-looping. See:
    www.jkp-ads.com/articles/noevents00.asp
    Jan Karel Pieterse
    Microsoft Excel MVP, WMVP
    www.jkp-ads.com
    Professional Office Developers Association

  3. #3
    Super Moderator
    Join Date
    May 2002
    Location
    Canberra, Australian Capital Territory, Australia
    Posts
    5,055
    Thanks
    2
    Thanked 417 Times in 346 Posts
    [quote name='SteveJeffrey' post='773890' date='06-May-2009 19:49']Hi all,

    My first post!

    Can I ask for suggestions on the code, and some opinions as to how to improve its efficiency or as a minimum stop it from looping?

    Many Thanks,

    -Steve[/quote]
    Hi Steve,

    Welcome to the Lounge!

    I'd be inclined to implement the code like:
    Code:
    Private Sub Worksheet_Calculate()
    Dim cl As Range
    For Each cl In Range("L9:L67")
      Application.EnableEvents=False
      If cl.Value = "ValueA" And UCase(Range("AB9")) = "ALLOW RUN" Then
    	cl.Offset(, 3).ClearContents
    	Range("AB9").ClearContents
      ElseIf cl.Value = "ValueB" And Range("T8") = 1 And UCase(Range("AC9")) <> "1st Set" Then
    	cl.Offset(, 9) = "1st"
    	Range("AC9") = "1st Set"
      End If
      Application.EnableEvents=True
    Next cl
    End Sub
    This way, you only need one IF test, instead of 4, and the cell updating won't cause recalculations during iterations of the loop (thus getting you stuck in an endless cycle) - only between them.

    One possible problem you'll still have is that, if cl.Offset(, 9) is ever set to "1st", then it never gets cleared if the conditions for setting it change. Conversely, if the conditions for cl.Offset(, 3).ClearContents occur, there's never any way to restore what was there previously if things change.
    Cheers,

    Paul Edstein
    [MS MVP - Word]

  4. #4
    New Lounger
    Join Date
    May 2009
    Posts
    2
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Hi Paul & Jan,

    Thanks for your comments, which have made all the difference to my code.
    I can see now how to structure the IF Statement to be more efficient, and it is now working exactly as it should be.

    Thanks for taking the time to respond - I really appreciate it!.

    Best regards,

    -Steve

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •