CODEWITHSUNDEEP
excelvba
jmaz
jmaz

Reputation: 527

Event Handler -- Cell not Updating as Expected

The code:

Private Sub Worksheet_change(ByVal target As Range)
Application.ScreenUpdating = False
Application.EnableEvents = False

Dim cell

For Each cell In Me.UsedRange.Columns("E").Cells
    If cell.Text = "Cu" And cell.offset(0, -1) = "WR229" Then
        MsgBox "Cu not permitted for WR229 or larger waveguide", vbOKOnly, "Cu Alert"
        cell = "Al"
    End If
Next cell

Application.ScreenUpdating = True
Application.EnableEvents = True
End Sub

The problem is that when the condition is met the cell does not get reset to the value "Al". Why not?

Upvotes: 0

Views: 61

Answers (3)

chris neilsen
chris neilsen

Reputation: 53166

One possible reason the OP's code can fail is if the UsedRange does not start in column A. This will occur if there is no data and no formatting at all in column A.

Why? Because .Columns (and .Rows and .Cells for that matter) are relative to the specified range. For example if the UsedRange is B2:Z10 then Me.UsedRange.Columns("E") will refer to range F2:F10.

Another issue in the OP's code is that it will run for any cells change, including those in Column A. This will throw an error because Offset -1 from Column A is invalid.

So, how to fix it? As answered by jbarker2160, you should take advantage of the Target parameter, which tells you which cells have changed. However that answer leaves several problems.

  • We want to check for Column E = "Cu" and Column D for "WR229", but we don't know which one will be entered first
  • We should account for the possibility that several cells are changed at once, eg due to a copy/paste
  • we should handle possible errors and not leave Events disabled
  • The OP's code is case sensitive. Ie "CU" will be accepted when "Cu" is not. Is this the desired behaviour? (If it is, remove the UCase$()'s from the code below)
  • Implicit in the OP's code is that there is a range of larger waveguides that are disallowed for Cu. This remains un-addressed in the code below.

This code addresses the above issues

Private Sub Worksheet_Change(ByVal Target As Range)
    Dim rw As Range

    On Error GoTo EH
    Application.ScreenUpdating = False
    Application.EnableEvents = False

    For Each rw In Application.Intersect( _
      Target.EntireRow, Me.UsedRange.EntireRow.Columns("D:E")).Rows

        If UCase$(rw.Cells(1, 2)) = "CU" And UCase$(rw.Cells(1, 1)) = "WR229" Then
            MsgBox "Cu not permitted for WR229 or larger waveguide", _
              vbOKOnly, "Cu Alert"
            rw.Cells(1, 2) = "Al"
        End If
    Next
EH:
    Application.ScreenUpdating = True
    Application.EnableEvents = True
End Sub

Upvotes: 1

Mr. Mascaro
Mr. Mascaro

Reputation: 2733

Code

Private Sub Worksheet_change(ByVal target As Range)
Application.ScreenUpdating = False
Application.EnableEvents = False

If target = "Cu" And target.offset(0, -1) = "WR229" Then
    MsgBox "Cu not permitted for WR229 or larger waveguide", vbOKOnly, "Cu Alert"
    target = "Al"
End If

Application.ScreenUpdating = True
Application.EnableEvents = True
End Sub

Explanation

Since you are doing this loop for each change, you don't need to loop through the entire column and using target will get around having to use the Value property.

Upvotes: 1

aphoria
aphoria

Reputation: 20209

Change this line:

cell = "Al"

To this:

cell.Value = "Al"

Upvotes: 0

Related Questions