Condensing repetitive code within an "If Statement"

user_9986773 0
Viewed 48

I have the following code, but after each condition for the "if" statement, the code is pretty much the same, except the first line. I was wondering if there was a way to reduce the code so that the code only runs through it only once.

For Each cel In AddOrRemoveRng
On Error Resume Next

If cel = "Add" And Not IsEmpty(cel.Offset(0, 1)) And Not cel.Offset(0, 3) = "0" Then
    AssetList.Cells(Asset_Row, Column_A).Value = "Add"
    AssetList.Cells(Asset_Row, Column_B).Value = cel.Offset(0, 3) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 4) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 5) 

    Asset_Row = Asset_Row + 1

ElseIf cel = "Remove" And Not cel.Offset(0, 3) = "0" Then
    TelecomAssetList.Cells(Asset_Row, Column_A).Value = "Retire"
    AssetList.Cells(Asset_Row, Column_B).Value = cel.Offset(0, 3) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 4) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 5)
    
    Asset_Row = Asset_Row + 1

ElseIf cel = "Update" Then
    TelecomAssetList.Cells(Asset_Row, Column_A).Value = "Update"
    AssetList.Cells(Asset_Row, Column_B).Value = cel.Offset(0, 3) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 4) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 5)
    
    Asset_Row = Asset_Row + 1


End If


Next cel
vba excel
2 Answers

Shortening Repetitive Code

  • Note that the add case uses AssetList, while the other two cases use TelecomAssetList.
  • I have replaced AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 5).Value with AssetList.Cells(Asset_Row, Column_D).Value = cel.Offset(0, 5).Value (_D instead of _C): it makes no sense to write twice to the same cell.
  • You can't use On Error Resume Next so 'loosely' and expect the code to work correctly. The statement is usually applied on a line or two with an appropriate 'closing follow-up',
    e.g. On Error Goto 0.
  • By using the DoWrite boolean, you are controlling if the 4 repetitive lines will be executed.
  • By using LCase and writing the cases in lower case, you are allowing lower/upper case typos, e.g. Add, adD and add are considered equal.
  • By using CStr, besides its main purpose, possible error values will not raise an error.
Dim DoWrite As Boolean

For Each cel In AddOrRemoveRng.Cells

    Select Case LCase(CStr(cel.Value))
    Case "add"
        If Not IsEmpty(cel.Offset(0, 1)) Then
            If CStr(cel.Offset(0, 3).Value) <> "0" Then
                AssetList.Cells(Asset_Row, Column_A).Value = "Add"
                DoWrite = True
            End If
        End If
    Case "remove"
        If CStr(cel.Offset(0, 3).Value) <> "0" Then
            TelecomAssetList.Cells(Asset_Row, Column_A).Value = "Retire"
            DoWrite = True
        End If
    Case "update"
        TelecomAssetList.Cells(Asset_Row, Column_A).Value = "Update"
        DoWrite = True
    End Select
    
    If DoWrite Then
        DoWrite = False
        AssetList.Cells(Asset_Row, Column_B).Value = cel.Offset(0, 3).Value
        AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 4).Value
        AssetList.Cells(Asset_Row, Column_D).Value = cel.Offset(0, 5).Value
        Asset_Row = Asset_Row + 1
    End If

Next cel
user_98140690

I believe this should help:

For Each cel In AddOrRemoveRng
On Error Resume Next

If cel = "Add" And Not IsEmpty(cel.Offset(0, 1)) And Not cel.Offset(0, 3) = "0" Then
    AssetList.Cells(Asset_Row, Column_A).Value = "Add"


ElseIf cel = "Remove" And Not cel.Offset(0, 3) = "0" Then
    TelecomAssetList.Cells(Asset_Row, Column_A).Value = "Retire"
    

ElseIf cel = "Update" Then
    TelecomAssetList.Cells(Asset_Row, Column_A).Value = "Update"


End If

    AssetList.Cells(Asset_Row, Column_B).Value = cel.Offset(0, 3) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 4) 
    AssetList.Cells(Asset_Row, Column_C).Value = cel.Offset(0, 5)

    Asset_Row = Asset_Row + 1

Next cel
user_177926680
Related