0

I have an Excel file from a third party that I wrote a macro to manipulate.

The macro needs to grab a technician's name and insert it prior to each service listed in the row per client.

The excel file is set up as follows.

|RowID|ClientID|CLname|CFname|TLname|TFname|Location|date|Serv|ServDur|ServID|Cost|

Each row has 1 unique client, multiple technicians, each technician can have multiple services.

My macro runs with no errors, yet it makes no changes.

Sub UpdateText()
' UpdateText Macro
' Keyboard Shortcut: Ctrl+Shift+D

Dim fName As String
Dim lName As String
Dim LastRow As Long
Dim LastCell As Long
Dim i As Long
Dim x As Long
LastRow = Cells(Rows.Count, 1).End(xlUp).Row

For i = 1 To LastRow
    LastCell = Range("IV1").End(xlToLeft).Column
    For x = 1 To LastCell
        If ActiveCell = "Location" Then
            lName = ActiveCell.Offset(0, -2).Value
            fName = ActiveCell.Offset(0, -1).Value
        ElseIf ActiveCell.Value Like "HC:H*" Then
            Selection.Insert Shift:=xlToRight, CopyOrigin:=xlFormatFromLeftOrAbove
            Selection.Insert Shift:=xlToRight, CopyOrigin:=xlFormatFromLeftOrAbove
            ActiveCell.Value = lName
            ActiveCell.Offset(0, 1).Value = fName
            ActiveCell.Offset(0, 3).Select
        End If
    Next x
Next i

End Sub

The active cell doesn't move along with the For statement. Meaning the Active Cell is always A1.

Community
  • 1
  • 1
Shane A. Workman
  • 112
  • 2
  • 16
  • 1
    Please try stepping through your code using F8 key to debug it and see what happens (or doesn't). Stepping through your code should at least allow you to describe where a specific failure mode occurs (e.g., evaluating an `If` statement should return `False` but it returns `True` instead and I can't figure out why, etc.). – David Zemens Oct 11 '16 at 17:09
  • RIght off the bat it's possible that your `lastRow` and `lastCell` are not what you think they are (step through & verify) and further complicating it is the fact that you're relying on `ActiveCell` while simultaneously inserting/shifting data in a loop, which seems immediately suspect. – David Zemens Oct 11 '16 at 17:11
  • The active cell doesn't move along with the For statement. Meaning the Active Cell is always A1. Other than ActiveCell what should I be using? – Shane A. Workman Oct 11 '16 at 17:22
  • Which `For` statement? There are two of them. One (the inner one) provides a condition whereby the active cell is offset (0,3). The outer `For` statement never changes the active row. I suggest revising this code to avoid reliance on `ActiveCell`. http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros – David Zemens Oct 11 '16 at 17:26

1 Answers1

0

Without changing anything structurally in your code it seems like you need to change the ActiveCell along with the For i loop:

For i = 1 To LastRow
    LastCell = Range("IV1").End(xlToLeft).Column
    For x = 1 To LastCell
        If ActiveCell = "Location" Then
            lName = ActiveCell.Offset(0, -2).Value
            fName = ActiveCell.Offset(0, -1).Value
        ElseIf ActiveCell.Value Like "HC:H*" Then
            Selection.Insert Shift:=xlToRight, CopyOrigin:=xlFormatFromLeftOrAbove
            Selection.Insert Shift:=xlToRight, CopyOrigin:=xlFormatFromLeftOrAbove
            ActiveCell.Value = lName
            ActiveCell.Offset(0, 1).Value = fName
            ActiveCell.Offset(0, 3).Select
        End If
    Next x
    Cells(i, 1).Select  '## Change your "ActiveCell" to the first cell in the next row.
Next i

This may yet not work 100% of the time because it's relying on ActiveCell and further, it's not explicitly activating anything, so that means it must rely on whatever the user had previously selected/activated before running the macro. This is unreliable, at best, because people forget to select the correct cell, etc...

Further: relying on ActiveCell leads to spaghetti code that becomes increasingly difficult to read, interpret, or troubleshoot (especially as you start working across multiple worksheets, workbooks, etc.) and defeats the purpose of object-oriented programming. Learn to use and manipulate the objects directly, and you'll find that it's very rarely (like, 0.01% of the time) necessary to Select or Activate anything:

Dim cl as Range '# Declaring a Range object to represent a cell on which to operate

For i = 1 To LastRow
    LastCell = Range("IV1").End(xlToLeft).Column
    For x = 1 To LastCell
        Set cl = Cells(i, x)   '## Assign your cell: row:=i, column:=x
        If cl.Value = "Location" Then
            lName = cl.Offset(0, -2).Value
            fName = cl.Offset(0, -1).Value
        ElseIf ActiveCell.Value Like "HC:H*" Then
            cl.Resize(1,2).Insert Shift:=xlToRight, CopyOrigin:=xlFormatFromLeftOrAbove
            cl.Value = lName
            cl.Offset(0, 1).Value = fName
            Set cl = cl.Offset(0, 3)
        End If
    Next x
    Set cl = Cells(i, 1).Select 
Next i

You may also want to review this for more reliable ways of obtaining the "last" row or column:

Error in finding last used cell in VBA

Community
  • 1
  • 1
David Zemens
  • 53,033
  • 11
  • 81
  • 130
  • 1
    It's worth noting, in order for it to work with your changes, while doing what i needed it to do, I had to add 2 to both LastCell and X. – Shane A. Workman Oct 11 '16 at 18:48
  • You can do that easily and optimally using `For i = 1 to LastRow Step 3` and `For x = 1 to LastCell Step 3` (instead of having to increment like `x = x +2` inside the loop. Cheers. – David Zemens Oct 11 '16 at 18:54