0

I have the following code in a macro in my personal book, and it works perfectly.

I'm trying to copy it into the actual wb it's running from so I can send it around for others to use, and it's breaking at the commented line "XXXXXX". The selected wb is being opened fine, but none of the subsequent editing occurs to that book. All of the following code (deletion of columns, etc) that should be happening to the opened workbook only happens to the wb running the macro, which is...sub-optimal.

I don't know why! Any thoughts welcomed.

Thank you

Sam

Sub PredictBoxValue()

Dim wb As Workbook
Dim myPath As String
Dim myFile As String
Dim myExtension As String
Dim FldrPicker As FileDialog
Dim TitleName As String
Dim sas As String
Dim sos As String
Dim unusedRow As Long
Dim filename As String

'Optimize Macro Speed
Application.ScreenUpdating = False
'Application.EnableEvents = False
'Application.Calculation = xlCalculationManual

'Retrieve Target Folder Path From User

sos = ActiveWorkbook.Name

ActiveSheet.Range("B11", "AF11").Clear

Dim fNameAndPath As Variant
fNameAndPath = Application.GetOpenFilename(FileFilter:="Excel Files (*.XLS),              
*.XLS", Title:="Select File To Be Opened")
If fNameAndPath = False Then Exit Sub
Set wb = Workbooks.Open(fNameAndPath)

sas = ActiveWorkbook.Name

'Delete extraneous columns and rows
'XXXXXXXXXXX
TitleName = Cells(5, 2).Value

Columns(8).Delete
Columns(12).Delete
Columns(12).Delete
Columns(3).Delete
Columns(2).Delete
Columns(1).Delete
Rows(3).Delete
Rows(2).Delete
Rows(1).Delete

Here:

Do Until Cells(2, 1).Value = "1"

Range("A1").End(xlDown).Select

'Do Until ActiveCell.Value = "1"
'ActiveCell.Offset(1).Select
'Loop

Do While ActiveCell.Value < 1
ActiveCell.EntireRow.Delete
ActiveCell.Offset(-1, 0).Select

Loop

ActiveCell.Offset(-1, 1).Select

Do While ActiveCell.Offset(0, -1).Value > 30
ActiveCell.EntireRow.Delete
GoTo Here

Loop

ActiveCell.Resize(, 7).Cut ActiveCell.Offset(1, 0).End(xlToRight).Offset(0,     
1)
ActiveCell.EntireRow.Delete

Loop

Rows(1).EntireRow.Delete

Cells(1, 1) = TitleName
Range("A1", Range("A1").End(xlToRight)).Copy

Windows(sos).Activate

ActiveSheet.Cells(11, 2).PasteSpecial (xlPasteValues)

Application.CutCopyMode = False


Windows(sas).Activate

'Save and Close Workbook
 wb.Close SaveChanges:=False

 Windows(sos).Activate

ActiveSheet.Cells(5, 3).Select

'Message Box when tasks are completed
MsgBox ("Data uploaded for ") & Range("B11")


ResetSettings:
  'Reset Macro Optimization Settings
Application.EnableEvents = True
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True


End Sub
Sam
  • 65
  • 1
  • 1
  • 9
  • 1
    You need to specify `ActiveWorkbook` to qualify all of the range references, such as `TitleName = ActiveWorkbook.Cells(5, 2).Value` and this needs to be done for each range reference (including the Columns(x) and Rows(x).Delete lines because those are also range references) `ActiveWorkbook.Columns(8).Delete` and so on. – tigeravatar Sep 28 '16 at 16:41
  • Thank you :) So activating a workbook does nothing then? I'd assumed I could activate the wb and then everything that followed would happen to that wb until another was activated. – Sam Sep 28 '16 at 16:50
  • 1
    Activating doesn't do too much - I ***highly*** recommend reading through, and applying, [how to avoid using `.Select`/`.Activate`](http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros) – BruceWayne Sep 28 '16 at 16:52
  • I've changed it to Activeworkbook.Columns(8).Delete and am getting an error - object doesn't support this property or method – Sam Sep 28 '16 at 16:54
  • Thanks BruceWayne, I will – Sam Sep 28 '16 at 16:54
  • @Sam `ActiveWorkbook.Sheets("Whatever sheet name you use").Columns...` or `ActiveSheet.Columns...` (but see BruceWayne's good advice, which is a better way!). – cxw Sep 28 '16 at 16:55
  • Thank you all, but no matter how I add in "ActiveWorkbook.Sheets("Sheet1").Co..." or "wb.activesheets.Co..." it's giving me the same error – Sam Sep 28 '16 at 17:01

1 Answers1

1

I took a bit of a closer look at your code. I think you are saying that you want to run code from macros.xlsm (or something like that), and have it operate on mydata.xlsx (or some such). Therefore, in your macro, ThisWorkbook will refer to macros.xlsm (should you need to refer to that).

After you have done Set wb = Workbooks.Open(fNameAndPath) to open mydata.xlsx, only and always refer to wb and wb.Sheets("whatever") when you are talking about mydata.xlsx.

  • Don't use Columns, Rows, Sheets, or Cells without a sheet reference in front of them
  • Don't use ActiveWorkbook/ActiveWorksheet at all.
  • Instead of ActiveCell, use a named range, e.g., as in this answer to the question BruceWayne noted.

That should take care of it!

<soapbox>And, in general, please be careful of your indentation and use longer variable names — both will help you avoid bugs as you work on this code.</soapbox>

Community
  • 1
  • 1
cxw
  • 16,685
  • 2
  • 45
  • 81
  • Thank you very much cxw. Is something wrong with "set ws = wb.Sheets("Sheet1")"?? I'm putting that in, but when I follow up with wb.ws.Columns(8).delete I'm hitting an error that's not there if I type out wb.Sheets("She...etc – Sam Sep 28 '16 at 17:17
  • @Sam once you have done `dim ws as Worksheet` and `set ws=...`, you can refer to `ws` by itself. The `ws` Worksheet object remembers what workbook it is associated with, so `ws.Columns(8).Delete` should work fine (leave off `wb.` at the beginning). – cxw Sep 28 '16 at 17:24
  • Ah! Brilliant. Thank you very much again :) – Sam Sep 29 '16 at 15:13
  • 1
    @Sam glad to help! As you'll recall from the tour, please hit the uparrow if an answer was helpful, and the checkmark if it solved your problem. That helps others who may read this question later. Plus, the checkmark clears the question off the "Unanswered" lists and gives us both reputation points :) . – cxw Sep 29 '16 at 16:13
  • Done :) Much obliged. – Sam Oct 26 '16 at 15:17